Subject: Comments on P1364.1/Draft 2.3, Clause 5
From: Shalom Bresticker (Shalom.Bresticker@motorola.com)
Date: Thu Oct 03 2002 - 08:05:33 PDT
5.1: Sentence 1 of para. 4 appears to duplicate sentence 1 of para. 2.
I suggest to combine para. 2 and para. 4 into para. 2, and then move
current para. 3 to after current para. 5, before Example 2.
5.1, para. 2: "It may be necessary to include in the event list ...":
How about wording it as a recommendation:
"However, all the variables read in the always statement should be
included in the event list in order to avoid mismatches between
simulation and synthesized logic."?
5.2.1.1-2: "always @(pos/negedge <clock_name>)":
<clock_name> implies a clock, which is misleading.
It could be a reset signal, for example.
5.2.2, 5.3: "A/n edge/level-sensitive storage device shall be modeled
for a variable ...".
That word "for" is confusing here.
Other places say, "modeled by" or "modeled using".
"modeled for" sounds like the meaning is different.
5.2.2: The document organization is confusing.
When I see that this section is about edge-sensitive storage devices,
I assume it refers to all kinds, both with and without async reset.
Then I read in the first sentence that only one edge event in the
event list is allowed, and I ask myself, "What about async reset??"
Only when I get to 5.2.2.1 do I see reference to async reset, and
then only after some more thinking do I understand retroactively
that 5.2.2 talks about ff's without async reset, not with async reset.
Note that 5.2.2 is a level-3 section heading, whereas 5.2.2.1 is a
level-4 heading, so I would expect 5.2.2.1 to be a sub-case of 5.2.2,
not a different case.
5.2.2: Para. 4 is not clear what it is talking about.
Add a reference to Example 11.
5.2.2, Examples 7-8: "edge-triggered" in the comments is redundant.
5.2.2, Examples 9-10: The "optionally" in the comments is confusing.
You want to clarify to say that the tool may choose a device with a
built-in sync set/reset input, if such a device exists in the cell
library.
Otherwise, it will do it via the D input. But currently,
the comment suggests that the tool may simply ignore the reset logic.
It is worthwhile that the note at the end of 5.2.2 be clearer and
come directly after these examples.
So I suggest that Example 11 be moved to after Example 8.
5.2.2, NOTE: The first sentence sounds like it means that you do not
have to use a specific style in order to infer a ff with sync s/r.
How about, "A synthesis tool is not required to infer an edge-sensitive
storage device with synchronous set/reset."
5.2.2.1, Para. 2: "if" and "else if" should be bold. That is
true throughout the document and also with respect to "always".
But it is particularly needed here.
5.2.2.1, Para. 3: "and or" should be "and/or".
5.2.2.1, Example 14: in last comment, "if" should be bold,
otherwise it is confusing.
5.2.2.1, Example 15: in first comment, "if" should be bold.
Second comment is redundant, it duplicates first one.
5.3: Document organization: 5.3 (level 2) is called
"Modeling level-sensitive storage devices",
but parallel subclause for edge-sensitive is 5.2.2 (level-3).
I propose:
Move 5.2.2 name to 5.2.
Delete 5.2.1, 5.2.1.1, 5.2.1.2 section headings, they are not needed.
Delete 5.2.2 section heading.
Then change 5.2.2.1 to 5.2.1.
5.3: "A ... device may be modeled ...":
"may be"? Should not that be "shall be"?
5.3: Para. 3, sentence 2: replace with copy of 5.2.2, para. 2, sentence
2.
5.2.2, para. 3: replace with copy of 5.3, para. 4.
5.3, Example 16: In comment, change "hold" to "retain".
5.4: Para. 2: "z assignments shall not propagate across ...
module instantiations".
TECHNICAL COMMENT: Really? Are you sure?
A module port connection normally acts like a short circuit.
If you are relying on 1364-2001, 5.6.6, DON'T!
We have an erratum on that section, that the description
is not completely accurate.
5.4, Example 20: "propogate" should be "propagate".
I still have comments on section 5.6-7 and 6.
Shalom
This archive was generated by hypermail 2b28 : Thu Oct 03 2002 - 08:14:11 PDT