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

ERROR: member top_flag_t of a packed union has 3 bits, expecting 24 #503

Closed QuantamHD closed 1 year ago

QuantamHD commented 1 year ago

When you uncomment,

// This one doesn't work
// typedef union packed {
//  foo_flags::common_flags_t  [3:0][7:0] atype_t;
//  padded_fooes_t     [3:0][7:0] btype_t;
// } top_flag_t;

in the example below you get the error ERROR: member bfloat16_flags of a packed union has 3 bits, expecting 24. There's a secondary bug here with the statement // b <= a.atype_t[0][0]; that also causes an error in the plugin. These seem like two seperate, but related issues.

The union members have the same size, but the plugin think they are different.

package foo_flags;
  typedef struct packed {
    logic a;
    logic b;
    logic c;
  } common_flags_t;
endpackage : foo_flags

package fooes;

  typedef enum logic [1:0]{
    a,
    b,
    c,
    d
  } classes;

endpackage : fooes

package goog;

typedef struct packed {
  logic a;
  fooes::classes b;
} padded_fooes_t;

// This one doesn't work
// typedef union packed {
//  foo_flags::common_flags_t  [3:0][7:0] atype_t;
//  padded_fooes_t     [3:0][7:0] btype_t;
// } top_flag_t;

// This one works
typedef union packed {
  foo_flags::common_flags_t atype_t;
  padded_fooes_t    btype_t;
} top_flag_t;

endpackage: goog

module top (
    input  clk,
    input  goog::top_flag_t a,
    output foo_flags::common_flags_t b
);

  always_ff @(posedge clk) begin
    b <= a.atype_t;
    // uncomment this one for the "doesn't work" case.
    // b <= a.atype_t[0][0];
  end

endmodule
QuantamHD commented 1 year ago

@kamilrakoczy @hzeller

kamilrakoczy commented 1 year ago

There is no information about ranges for atype_t in UHDM. I've created issue in Surelog to fix this: https://github.com/chipsalliance/Surelog/issues/3642

wsipak commented 1 year ago

The issue with missing ranges has been fixed in Surelog. It appears that bumping Surelog to 8b94787 also requires some changes in the plugin. Here's a change that adjusts the way sizes of constant objects are interpreted (merged): https://github.com/chipsalliance/yosys-f4pga-plugins/pull/510

We're working on making use of the new information about ranges that is now added to typespecs in Surelog 8b94787.

kamilrakoczy commented 1 year ago

@QuantamHD This issue should be fixed using submodules after: https://github.com/antmicro/yosys-systemverilog/pull/1710.

kgugala commented 1 year ago

looks like GH closed this automatically, @QuantamHD can you check if this works for you, and close the issue if everything is OK?

QuantamHD commented 1 year ago

Will do @grotival heads up

grotival commented 1 year ago

works well! Thanks!

kgugala commented 1 year ago

great, so we can close this