Qucs / ADMS

ADMS is a code generator for the Verilog-AMS language
GNU General Public License v3.0
94 stars 32 forks source link

Cannot parse multiple attribute instances #105

Closed ngwood closed 2 years ago

ngwood commented 2 years ago

Version 2.3.0 of the Verilog-AMS Language Reference Manual introduced multiple attribute instances; e.g.,

(* desc = "length", units = "meters" *)

and

(* desc = "length" *) (* units = "meters" *)

are equivalent.

ADMS cannot currently handle the latter form.

ngwood commented 2 years ago

A solution to this limitation is as follows.

$ git diff verilogaYacc.y.in 
diff --git a/admsXml/verilogaYacc.y.in b/admsXml/verilogaYacc.y.in
index fc7b7a3..89980f7 100644
--- a/admsXml/verilogaYacc.y.in
+++ b/admsXml/verilogaYacc.y.in
@@ -278,7 +278,11 @@ R_s.nature_assignment
         ;
 R_d.attribute.0
         |
+        | R_d.attribute.1
+        ;
+R_d.attribute.1
         | R_d.attribute
+        | R_d.attribute.1 R_d.attribute
         ;
 R_d.attribute
         | tk_beginattribute R_l.attribute tk_endattribute
@@ -370,7 +374,7 @@ R_s.declaration.withattribute
           _ set_context(ctx_moduletop);
         ;
 R_d.attribute.global
-        | R_d.attribute
+        | R_d.attribute.1
           _ gGlobalAttributeList=gAttributeList;
           _ gAttributeList=NULL;
         ;
@@ -1190,7 +1194,7 @@ R_s.assignment
           _ Y($$,(p_adms)myassignment);
           _ myvariableprototype->_vcount++;
           _ myvariableprototype->_vlast=myassignment;
-        | R_d.attribute tk_ident '=' R_s.expression
+        | R_d.attribute.1 tk_ident '=' R_s.expression
           _ p_assignment myassignment;
           _ p_variable myvariable=variable_recursive_lookup_by_id(gBlockList->data,$2);
           _ p_variableprototype myvariableprototype;
@@ -1224,7 +1228,7 @@ R_s.assignment
           _ Y($$,(p_adms)myassignment);
           _ myvariableprototype->_vcount++;
           _ myvariableprototype->_vlast=myassignment;
-        | R_d.attribute tk_ident '[' R_expression ']' '=' R_s.expression
+        | R_d.attribute.1 tk_ident '[' R_expression ']' '=' R_s.expression
           _ p_assignment myassignment;
           _ p_array myarray;
           _ p_variable myvariable=variable_recursive_lookup_by_id(gBlockList->data,$2);

I used the following test code to verify the attributes would parse correctly; this included various combinations standard and ADMS non-standard attribute positions.

// dummy.va

`include "constants.vams"
`include "disciplines.vams"

module dummy(a, b);
    inout a, b;
    electrical a, b;

    parameter real c = 0.0;
    parameter real d = 0.0 (* desc = "D1" *);
    (* desc = "E1" *) parameter real e = 0.0;
    (* desc = "F1" *) parameter real f = 0.0 (* desc = "F2" *);
    parameter real g = 0.0 (* desc = "G1" *) (* desc = "G2" *);
    (* desc = "H1" *) (* units = "H2" *) parameter real h = 0.0;
    (* desc = "I1" *) (* units = "I2" *) parameter real i = 0.0 (* desc = "I3" *) (* units = "I4" *);
endmodule

I then used Xyce's html_params.xml template to verify the attributes were being processed correctly.

I only tested this with parameters, but the change will correctly update everything that can have an attribute.

What I discovered during the process of testing this enhancement was interesting and not a result of this changed; this is that ADMS is not discarding duplicate attribute definitions. The LRM states that

If the same attribute name is defined more than once for the same language element, the last attribute value shall be used and a tool can give a warning that a duplicate attribute specification has occurred.

You could argue that this is a feature, but I would argue otherwise.

ngwood commented 2 years ago

cadf421 fixes this issue.