[sv-ec] RE: Mantis 2506

Hi Shalom:

Thanks for the comments. I apologize for the slow response. I was out of the office last week. See my comments below. I have uploaded the new version to mantis.

Thanks,
Scott

From: owner-sv-ec@eda.org [mailto:owner-sv-ec@eda.org] On Behalf Of Bresticker, Shalom
Sent: Monday, September 19, 2011 4:49 AM
To: SV_EC List
Cc: sv-champions@eda.org
Subject: [sv-ec] RE: Mantis 2506

Any comments?

From: owner-sv-ec@eda.org [mailto:owner-sv-ec@eda.org] On Behalf Of Bresticker, Shalom
Sent: Wednesday, September 14, 2011 9:26 PM
To: SV_EC List
Subject: [sv-ec] Mantis 2506

Hi,

I started reviewing Mantis 2506 for the Champions. I found a few issues, some of which I believe are mandatory to fix. I will be on vacation for the next few days, so I wanted to send those issues I already saw. I have gotten a little more than halfway through the proposal so far.

  1. First, the Mantis page has in the Additional Information field: "Approved on sv-ec meeting 5/23/2011,. The proposal:
Proposal for Mantis 2506_v6+friendly_amendments.pdf", whereas the current proposal is v9. The comment should be either deleted or updated.
 [SL] Updated

  1. On page 4, it says, "The initializers for such instance constants shall appear before the referring covergroup constructor call in the class constructor." Steven Sharp had an issue with "appear" in Mantis 2900. See http://www.eda.org/sv-ec/hm/7984.html and following. Is the issue relevant here too? I'm not sure.
 [SL] That issue isn't relevant here. The idea here is to impose restrictions such that the instance constant is guaranteed to be initialized prior to covergroup construction.

  1. Page 4: "Functions shall be automatic, preserve no state information, and have no side effects." Everywhere else in the LRM, the wording is "automatic (or preserve no state information)": 16.6, 17.8, 18.5.11, and Mantis 3398. Why be different here? Shouldn't they all be same, one way or the other?
 [SL] Made to be consistent with the rest of the LRM. Mantis item 3625 has been opened to address this language.

  1. Page 4: "User-defined system task or function calls are restricted to constant system function calls (see 11.2.1)". I don't understand this.
"User-defined system task or function calls" refers to PLI functions. Constant system function calls refers to certain built-in system functions. How do they go together?

[SL] Removed user-defined.

  1. Page 4: "bit [7:0] coverpoint y[31:24]; // creates coverpoint "y" covering the high" "bit" should be bold.
 [SL] Fixed.

  1. Page 4: "cross x, y { // Creates implicit coverpoint "y" covering" Coverpoint y was already created above, in the new line. One of the lines needs to be changed.
 [SL] Named the coverpoint d and renamed the subsequent coverpoint e.

  1. Page 5, bottom: "It shall be legal to use the $ primary in an covergroup_value_range of the form [ expression : $ ] or [ $ : expression ]." "an" needs to be "a".
 [SL] Fixed.

  1. Footnote 37 in Draft 2 BNF says, "37) The $ primary shall be legal only in a select for a queue variable, in an open_value_range, or as an entire sequence_actual_arg or property_actual_arg." "covergroup_value_range" needs to be added to the list.
 [SL] This is found in v9 in the changes to A.10. Does it need to be changed elsewhere?

  1. In the BNF, "open_value_range ::= value_range" has a footnote "23) It shall be legal to use the $ primary in an open_value_range of the form [ expression : $ ] or [ $ : expression ]." A similar footnote needs to be added to covergroup_value_range.
 [SL] Added an appropriate footnote.

  1. Page 6: "By default the with_covergroup_expression is applied to the set of values in the covergroup_range_list prior to distribution of values to the bins." Add comma after "default".
 [SL] Done.

  1. Page 6: "If the distribution of values is desired before with_covergroup_expression application the distribute_first covergroup option (see 19.7.1) can be used to achieve this ordering." Add comma after "application".
 [SL] Done.

  1. Page 6: "The result of applying a with_covergroup_expression shall preserve multiplicity of bin items as well as bin order." Change "multiplicity" to something which is less likely to not be understood. Many people will not understand what this is trying to say.
 [SL] Replaced with "multiple, equivalent".

  1. Pages 8-9: "An implementation shall issue a warning under the following conditions if a coverpoint type is not present:
1) If e is unsigned and b is signed with a negative value.
2) If assigning b to a variable of type type(e) would yield a value that is not equal to b under normal comparison rules for ==.
3) If b yields a value with any x or z bits. This rule does not apply to wildcard bins because x and z shall be treated as 0 and 1 as described above.
An implementation shall issue a warning under the following conditions if a coverpoint type is present.
1) If the coverpoint type is unsigned and b is signed with a negative value.
2) If assigning b to a variable of coverpoint type would yield a value that is not equal to b under normal comparison rules for ==.
3) If b yields a value with any x or z bits. This rule does not apply to wildcard bins because x and z shall be treated as 0 and 1 as described above."

It looks like the two lists are identical except that the first refers to type(e) and the second to the coverpoint type. Can't you combine them instead of repeating all that text? (If you leave them, then you need a colon after "if a coverpoint type is present".)

[SL] I believe that the second set can be removed. a and b above define the type for e and b under both conditions. Given that information only a single set of rules should be needed. Do you agree?

  1. Page 9: "If an element of a bins covergroupopen_range_list is a range [b1 : b2] and there exists at least one value in the range for which a warning would not be issued then the range shall be treated as containing the intersection of the values in the range and the values expressible by type(e)." Should the end refer to "values expressible by a variable of the coverpoint type" instead of "type(e)"?
 [SL] Given the fact that e shall be statically cast to the coverpoint type (from a above), is there a difference between "values expressible by a variable of the coverpoint type" and "type(e)"? I believe that type(e) covers both cases and is therefore the right thing to have there.

  1. Page 10: "list_of_coverpoints ::= cross_item , cross_item { , cross_item }" I know this is unchanged from the current LRM, but if each item in the list is a cross_item, doesn't it make more sense to call the list "list_of_cross_items" or something similar?
 [SL] Changed to list_of_cross_items.

  1. Page 11, bottom:" 19.6.1: Defining cross coverage bins" Delete the colon.
[SL] Done.

Regards,
Shalom

Shalom Bresticker
Intel LAD DA, Jerusalem, Israel
+972 2 589 6582 (office)
+972 54 721 1033 (cell)
http://www.linkedin.com/in/shalombresticker

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

--
This message has been scanned for viruses and
dangerous content by MailScanner<http://www.mailscanner.info/>, and is
believed to be clean.
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
--
This message has been scanned for viruses and
dangerous content by MailScanner<http://www.mailscanner.info/>, and is
believed to be clean.
-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.