chipsalliance / yosys-f4pga-plugins

Plugins for Yosys developed as part of the F4PGA project.
https://f4pga.org
Apache License 2.0
80 stars 46 forks source link

part of the packed struct parameter read as 0s #482

Closed grotival closed 1 year ago

grotival commented 1 year ago

If I read this verilog with UHDM + Yosys and write to verilog I get a different value than the packed struct parameter.

package my_package;

  typedef struct packed {
    logic a;
    logic b;
  } my_struct;

  parameter my_package::my_struct my_parameter = '{
    a: 1'b1,
    b: 1'b1
  };

endpackage : my_package

module my_module (
    output my_package::my_struct z
);

assign z = my_package::my_parameter;

endmodule : my_module

I get:

module my_module(z);
  output [8:0] z;
  wire [8:0] z;
  assign z = 2'h1;
endmodule

Would anyone get a chance to TAL at this? That would be very much appreciated!

FYI: The full example I need is with my_package living in a different file, and my_parameter living in another package and a different file from my_package. I can't test that yet.

grotival commented 1 year ago

@QuantamHD @kamilrakoczy This would be super helpful for Google.

tgorochowik commented 1 year ago

Thanks for the report, we'll take a look at this

mandrys commented 1 year ago

The issue was caused by the fact that only a value of the first struct field was added to the whole parameter value, and the rest was ignored - it looked like this because of skipping the string of the fields when adding the values of struct, so all of them had the same name, my_package::my_parameter, hence they were treated as one field. It will be fixed in the PR https://github.com/chipsalliance/yosys-f4pga-plugins/pull/488.

Also, there was another issue spotted, that is linked to the reported one, where accessing the struct fields from the parameter like this:

assign x = my_package::my_parameter.a;
assign y = my_package::my_parameter.b;

gives the values of the whole parameter, instead of the single bit value. The field name is missing in UHDM already, so this issue has been reported in Surelog: https://github.com/chipsalliance/Surelog/issues/3604.

mandrys commented 1 year ago

The fix has been merged. Closing the issue.