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.
2. 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.
3. 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?
4. 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?
5. Page 4: "bit [7:0] coverpoint y[31:24]; // creates coverpoint "y" covering the high" "bit" should be bold.
6. 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.
7. 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".
8. 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.
9. 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.
10. 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".
11. 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".
12. 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.
13. 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".)
14. 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)"?
15. 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?
16. Page 11, bottom:" 19.6.1: Defining cross coverage bins" Delete the colon.
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, and is believed to be clean.Received on Wed Sep 14 11:26:33 2011
This archive was generated by hypermail 2.1.8 : Wed Sep 14 2011 - 11:26:45 PDT