editorial comments on draft 8

Subject: editorial comments on draft 8
From: Paul Graham (pgraham@cadence.com)
Date: Mon May 06 2002 - 14:26:15 PDT

I have some comments on SystemVerilog draft 8. I'm looking at the
unannotated copy, which is what users will eventually see.

As you can tell, I hadn't looked over the lrm very closely before :-(

---

3.6 Enumerations

Third paragraph:

    An enumerated type has one of a set of named values.

I don't understand what this means. How about:

    An enumerated type defines a set of named values.

Or is the intent to say:

    A variable of an enumerated type holds one of a set of named values.

As a matter of style, I think it would be helpful to distinguish between
types and variables (or declared objects). I think of a type as something
like bit, real, logic, etc., and a variable as a wire, reg, or parameter
which is declared to be of some type.

I see that the glossary entry for enumerated type has phrasing similar to
what I am suggesting. 

---

3.7 Structures and Unions

    Named structure types must always use typedef, as there is no equivalent
    of the C struct adjective, such as struct instruction IR;.

I'm not sure what this sentence means.  Does it mean that a struct name
cannot be used to declare a variable, but may only appear in a type
definition?  I say delete this sentence.

    A structure can be assigned as a whole, and passed to or from a function
    or task as a whole. Note that it is inefficient to copy large
    structures.

What is the point of the second sentence in this paragraph?  Is it a
guarantee that an implementation will copy structs and unions inefficiently?
Or is it a statement of the obvious fact that it takes longer to copy 2N
bytes than it does to copy N bytes?  If the latter, then it has no place in
a language specification!  We might as well mention that it's inefficient to
multiply two 16Kbit vectors.

---

3.8 Casting

First paragraph:

    A data type may be changed by using a cast ( ' ) operation. The expression
    to be cast must be enclosed in parenthesis or within concatenation or
    replication braces.

Typo -- should be "parentheses" instead of "parenthesis".

    A complex data type cannot be used. It must be defined with a typedef.

This paragraph only restates the BNF.  It seems to be saying that you can't
have a cast like this:

    struct { int a; bit b; } ' (1);

But this is obvious from the BNF, so I see no need to restate it in the
text.  In general, the text should restrict the language permitted by the
bnf, not repeat the restrictions.  This sentence is like saying "a complex
data type may not be used as a module name; only an identifier may be used".

And the first sentence is wrong anyway, since apparently the name of a
complex data type can be used as the simple type.  Which makes the name
'simple_type' somewhat contradictory.

I think what you mean is that the type prefix must be the name of a type (or
a number).  The type prefix cannot be the definition of a type.

I would vote to delete this paragraph.

For the example at the end of 3.8, there should be a declaration of "a".
The example has:

    t = tagbits'(a[3]); // convert structure to array of bits
    a[4] = tagged'(t); // convert array of bits back to structure

and I suppose that a is an array (packed or unpacked) of type tagged.  Why
not add a declaration:

    tagged a[4];

---

4.2 Packed and unpacked arrays

    If a packed array is declared as signed, then the array viewed as a
    single vector shall be signed, as in Verilog 2001. Note that Verilog
    2001 defines that a part selects of an array is unsigned.

The second sentence in this paragraph is not necessary.

It should be mentioned that when a packed array is declared as 'signed',
that 1) the individual elements of the array are signed, and 2) the array as
a whole, when viewed as a bit-vector, is signed.  

---

5.0 Introduction

In the second and third paragraphs it is stated that C and Verilog macros
are constants.  They really aren't constants in the sense of being constant
data objects, so I would drop macrso from the list of constant items in
these two paragraphs.

---

5.3 Constants

The next-to-last paragraph reads:

    A constant expression contains literals and other named constants.

How about

    A constant expression may only refer to literals and other named
    constants.

---

6.2 Attribute syntax for interfaces

There is a typo, a missing newline, in Syntax 6-1, between
interface_declaration and interface_item.  interface_item appears at the end
of the last line of interface_declaration.

Just a suggestion, but the bnf in 6-1 would be more readable if the entire 
syntax for interface_declaration were not repeated.  Instead of:

    interface_declaration ::= 
        { attribute_instance } interface interface_identifier [ parameter_port_list ] 
	[ list_of_ports ] ; [unit] [precision] { interface_item } 
	endinterface [: interface_identifier] 
	| { attribute_instance } interface interface_identifier [ parameter_port_list ] 
	[ list_of_port_declarations ] ; [unit] [precision] { non_port_interface_item } 
	endinterface [: interface_identifier] 

how about:

    interface_declaration ::= 
        { attribute_instance } interface interface_identifier [ parameter_port_list ] 
	[ port_list ] ; [unit] [precision] { interface_item } 
	endinterface [: interface_identifier] 

    port_list ::=
          list_of_ports
	| list_of_port_declarations

Then mention in the text that if list_of_port_declarations appears then
interface_item may not include input/inout/output declaration. 

Also note that the bnf in 6-1 is similar, but not identical, to the syntax
in A.1.3.  In A.1.3, "[unit] [precision]" is combined into
"[timeunits_declaration]".

I know that the syntax here follows the Verilog-2001 pattern of duplicating
large blocks of syntax for module (and task and function).  But the point of
the bnf is to be readable, not to fully specify the language.  I don't find
the existing 6-1 syntax box to be very readable.  I have to trace the two
productions in parallel to find that they differ in two, and only two,
places.

And to be pedantic, notice that the grammar for interface_declaration is
ambiguous -- if list_of_ports/list_of_port_declarations is omitted, there is
no way to tell from the grammar which alternative is correct.

---

Section 7.2

Table 7-2 lists the event trigger "->" as an operator.  It's really not an
operator at all, any more than '(' is an operator.

Also, the note at the bottom of table 7-2 says that "& is [sic] a higher
precedence than ^".  Shouldn't "&" be in a different line of the operator
precedence table than "^"?  And where does the precedence of "|" fit in --
with "&", with "^", or in between?

---

Section 8.1

Table 8-1 follows the Verilog-2001 LRM in its awesome duplication of
"{attribute_instance}".  Instead of:

    statement ::= [ block_identifier : ] statement_item

    statement_item ::= 
	    { attribute_instance } blocking_assignment ; 
	    | { attribute_instance } nonblocking_assignment ; 
	    | { attribute_instance } procedural_continuous_assignments ; 
	    | { attribute_instance } case_statement 
	    | { attribute_instance } conditional_statement 
	    | { attribute_instance } transition_to_state statement_or_null 
	    | { attribute_instance } inc_or_dec_expression 
	    | { attribute_instance } function_call /* must be void function */ 
	    | { attribute_instance } disable_statement 
	    | { attribute_instance } event_trigger 
	    | { attribute_instance } loop_statement 
	    | { attribute_instance } jump_statement 
	    | { attribute_instance } par_block 
	    | { attribute_instance } procedural_timing_control_statement 
	    | { attribute_instance } seq_block 
	    | { attribute_instance } system_task_enable 
	    | { attribute_instance } task_enable 
	    | { attribute_instance } wait_statement 
	    | { attribute_instance } process statement 
	    | { attribute_instance } proc_assertion

why not:

    statement ::= [ block_identifier : ] { attribute_instance } statement_item

    statement_item ::=
	    blocking_assignment ; 
	    | nonblocking_assignment ; 
	    | procedural_continuous_assignments ; 
	    | case_statement 
	    | conditional_statement 
	    | transition_to_state statement_or_null 
	    | inc_or_dec_expression 
	    | function_call /* must be void function */ 
	    | disable_statement 
	    | event_trigger 
	    | loop_statement 
	    | jump_statement 
	    | par_block 
	    | procedural_timing_control_statement 
	    | seq_block 
	    | system_task_enable 
	    | task_enable 
	    | wait_statement 
	    | process statement 
	    | proc_assertion

Isn't that more readable?  It also makes it easier to change the grammar
later.  For instance, if we want to allow only zero or one
attribute_instances instead of zero more, we don't have to change the
grammar for statement in a dozen places.

The same transformations could be applied to function_statement_item,
restricted_statement_item, etc.

---

Section 8.3 Selection statements

    In Verilog, an if (expression) is evaluated as a boolean, so that if the
    result of the expression is 0 or X, the test is considered false. With
    SystemVerilog, null or void or {} are also false.

Is it true that {} is a valid expression in SystemVerilog?  I don't see how
the bnf allows this.  A concatenation has to have at least one expression
within the braces.

---

Section 8.9 Event control

    SystemVerilog adds an iff qualifier to the @ event control. 

        module latch (output logic [31:0] y, input [31:0] a, input enable); 
	    always @(a iff enable == 1) 
	        y <= a; // latch is in transparent mode endmodule

    The event expression only triggers if the expression after the iff is
    true, in this case when enable is equal to 0.

The example checks for enable == 1, but the following text says that it
checks for enable == 0.  The two should be reconciled.

---

Section 10.2 Tasks

(As with interfaces, the bnf for task_declaration is repeated, but I won't
dewll on it.)

There is no description in section 10.2. of task_prototype.  There is no
description of the 'port' keyword.  There is no description of an 'event'
variable.  These missing descriptions should be filled in or at least
referenced here.

---

Paul

This archive was generated by hypermail 2b28 : Mon May 06 2002 - 14:28:28 PDT