RE: [sv-ac] review of D4 implementation of 1674 implemen

 
Hello John,

please see below.

ed

> -----Original Message-----
> From: owner-sv-ac@eda.org [mailto:owner-sv-ac@eda.org] On 
> Behalf Of John Havlicek
> Sent: Saturday, October 13, 2007 2:47 PM
> To: eduard.cerny@synopsys.COM
> Cc: sv-ac@eda.org
> Subject: [sv-ac] review of D4 implementation of 1674 implemen
> 
> Hi Ed:
> 
> I looked through your comments on 1674.  I have some comments
> and questions.  Please see the "JH" marks below.  I will look
> for your answers and feedback before sending this on to Stu.
> 
> J.H.
> 
> 
> ==============================================================
> =====================
> 1674 [EC] (consider reference to 1648)
> 
> - Reveiwed by EC 2007-10-05.
> 
> - in the text
> 
>      The inferred enable condition is the expression defining the
>      cumulative condition required to reach the current point within
>      enclosing if-else and case blocks statements inside an always or
>      initial block procedure.  This is the same as the contextually
>      inferred enabling condition for verification statements (see
>      16.14.5). If there are no enclosing if-else or case blocks
>      statements, then the cumulative condition is 1'b1 (true).
> 
>   should the if-else, case, always and initial be bold as keywords, as
>   indicated in the proposal?
> 
>   JH:  The editor's note makes it clear that the font/bolding was
>   adjusted for consistency.  We should ask for a statement of the 
>   rationale so that we can follow it in the future.

You may send my comment to him and he can decide...

> 
> - There is reference to default disable (1648), but this was not
>   approved at the time this change was implemented. The proposal for
>   1648 was updated following the most recent comments and should thus
>   be voted ASAP.
> 
>   JH:  We discussed this in the meeting on 2007-10-09.  I do not 
>   think that any implementation feedback on this item is needed.
>   We agreed to enter another Mantis item whose purpose will be fix
>   this if 1648 is ultimately rejected.

OK

> 
> - in 
> 
>      a4: assert property(p_multiclock(negedge clk2, ,posedge 
> clk1, a, b, c, d);
> 
>   there should be a space between "," and "posedge clk1", as in
> 
>      a4: assert property(p_multiclock(negedge clk2, , posedge 
> clk1, a, b, c, d);
> 
>   JH:  This is not essential, but it makes the comma/space 
> conventions more
>   consistent across the examples.

That's what I think too.

> 
> - in
> 
>      always @(posedge clk2 or posedge rst) begin
>        if (rst) ... ;
>        else if (d)
>      end
> 
>   perhaps it should be changed to
> 
>      always @(posedge clk2 or posedge rst) begin
>        if (rst) ... ;
>      end
> 
>   because the "else if (d)" is just dangling there. This 
> error is in the
>   proposal, but I think the correction should be made.
> 
>   JH:  These changes are for the rewritten module m.

Yes, but I think thet it then should be 

     always @(posedge clk2 or posedge rst) begin
       if (rst) ... ;
       else if (d) ;
     end

or something like that, no? 

> 
> 
> - in
> 
>      Assertion a2 uses explicit reset value '0 in which case the
>      disable iff statement could be omitted altogether in the
>      equivalent assertion.
> 
>   '0 should be changed to 1'b0.  (The same error is in the proposal.)
> 
>   JH:  Since the error was in the proposal, we should enter a 
> new Mantis item.

Is it really necessary for such a small editorial change? it could be
even changed to the simpler 'b0.

> 
> - in
> 
>     else block of the if (rst) statement and d is from the if block
>     statement and thus not negated.
> 
>   should "else" be in bold as a keyword?
> 
>   JH:  The editor seems to be implementing some convention 
> for uniformity.
>   We should ask about the rationale.

He can decide...

> 
> - Question: in several places we use the type bit as a cast function,
>   since bit is also a keyword, should it be bold in all its
>   occurrences?  E.g., (!bit'(rst!=1'b0) && d)
> 
>   JH:  Which specific occurrences need to be bold?

The following ones:

a3: assert property
(
@(posedge clk2) disable iff (rst1)
(!bit'(rst!=1'b0) && d) |->
(!bit'(rst!=1'b0) && d) throughout (a ##0 b[->1]) |=> c
);

and in the last paragraph of the proposal:

(!bit'(rst!=1'b0) && d)

These were not bold in the proposal either, but since he does the
unifromisation... hopefully he cand decide what it should look like w/o
a new mantis item.




> 
> 
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
> 
> 

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.