Support for attributes on parameters in Verilog

[mkurc-ant]

This PR adds support for attributes on parameters for Verilog.

The Verilog frontend was augmented with parsing and storing attributes of parameters in their AST nodes. Parameter attributes are only stored for those that actually have them. The default value of a parameter is also preserved.

[mithro]

Do you need to update the chparam and setparam + equivalent commands to use the new iterators for parameter lists?

[mkurc-ant]

@mithro I didn't do anything to chparam and setparam because the actual parameter values are stored somehow differently. I'think there is no need to change that.

[mkurc-ant]

Now the RTLIL::Module class holds:

pool<RTLIL::IdString> avail_parameters;
dict<RTLIL::IdString, RTLIL::ParameterInfo> parameter_information;
dict<RTLIL::IdString, dict<RTLIL::IdString,RTLIL::Const>> parameter_attributes;

The functionality regarding avail_parameters is unchanged. The structure parameter_information is meant to hold "metadata" of a parameter (default value, width etc.) Currently only the default value is stored.

The dict parameter_attributes contains attributes of parameters but only for those which actually have them.

I also updated the JSON, ilang and Verilog backends. Attributes of parameters are now visible when using the dump command.

[litghost]

Looking good. Two general thoughts:

• Are there tests we should be adding?
• Can we flag out the "cost" of these changes?

[litghost]

I don't believe this is correct? You can use escape identifiers to write them out?

[mkurc-ant]

Fixed.

[mkurc-ant]

@litghost I already added some tests regarding parsing of parameter attributes and they are merged upstream. I suppose I should add additional tests which will include eg. reading Verilog, writing JSON/ILANG and comparing output with a reference. I haven't found any similar tests in the Yosys tests folder. What do you think ?

Regarding the "cost": I believe that the cost can only be observable when processing verilogs with huge number of attributes on parameters. I would need to think of some kind of memory usage and performance tests. I haven't found such tests either.

[litghost]

LGTM

[mkurc-ant]

@litghost Do not merge. I recently discovered that floating point constants are handled differently than strings and integers. Also a floating point constant is a different object in the AST tree. I added a quick workaround which treats floating point constants as integer 0 but it is not an acceptable solution.

In order to fix that I would need either to refactor the RTLIL::Const class so it can store a float or the newly added RTLIL::ParameterInfo class which holds default value of a parameter. I think the latter will be less memory consuming.

Anyway, there is still some work to do with this PR.

[litghost]

@litghost In order to fix that I would need either to refactor the RTLIL::Const clas

Maybe we should not modify Yosys and simply extract the FASM_PARAM annotation using a coarse parser. I'm not a fan of keeping a Yosys fork forever, and the more that has to change, the less likely they will accept it. @mithro, thoughts?

[mkurc-ant]

Anyway I managed to do the fix. So the default value of a parameter can now be floating point. It is correctly loaded by the Verilog frontend and correctly outputted by the Verilog and the JSON backend.

The ilang frontend never parsed default values of parameters so I left its functionality intact.

[mithro]

LGTM.

[mithro]

@litghost Let's land this and then we can figure out an alternative approach.

[mithro]

The attribute is specified as a prefix to a declaration, a module item, a statement or a port connection. Three examples are:

• (* optimize *) module control ( ... );
• (* state_variable *) reg [7:0] S;
• dff il (q, d, (* clock_line *) ckl, rst);

The attribute is specified as a suffix to an operator or the function name in a function call.

• sum = a + (* ripple_adder *) b;
• assign next_state = fsm_func(* meally_fsm *) (state, a, b, c);

[mithro]

ordered_port_connection ::= (From A.4.1)
{ attribute_instance } [ expression ]
named_port_connection ::=
{ attribute_instance } . port_identifier ( [ expression ] )

[mithro]

@mkurc-ant So, it appears you missed being able to add an attribute to a port connection. See the third item above. Could you fix that?

[eddiehung]

I've discussed this issue with Yosys upstream and we agree this is a good idea, however, the way we'd suggest implementing it would be to transform the parameter into a new wire (permanently) driven by the parameter value. That way, it appears post-elaboration, and in the backend too, so that it can be inspected during simulation or used during formal verification too.

Would you consider trying this approach instead?