SV interface name collision in different parts of design hierarchy -- Verilator muddles names between levels of hierarchy

[paul-demo]

When I have an array of SV interfaces like this, Verilator works as expected for N=4.

AXIS_IF.master m_axis_if[N-1:0]

However, when I change to N=1, I get errors where Verilator has apparently mixed up master/slave modports and thinks I am assigning to an sv signal with the wrong direction. Basically Verilator seems to have confused master/slave modport determination, which usually automatic based on what the modport is connected to (ex: an AXIS_IF.master or AXIS_IF.slave modport of some module should and does cause it to pick the correct modport of the interface when N > 1).

  AXIS_IF #(
      .DATA_W(AXIS_DATA_WIDTH),
  ) axis_if (
      .aclk   (clk),
      .aresetn(~reset)
  );
  assign axis_if.tdata = tdata;

This produces errors like:

Attempt to drive input-only modport: 'tdata'
  144 |   assign axis_if.tdata = tdata;

In this example, tdata is an input and axis_if is not even exposed as a port of the module.

[wsnyder]

We will need a complete, small example, thanks.

One thing to try is change the default parameter value for N in the interface definition.

[paul-demo]

I will try to reproduce with something less than my full design.

The interface definition is not arrayed and is basically just what you would expect:

/*
  AXI-Stream Interface
*/
interface AXIS_IF #(
    parameter int DATA_W = 128,
    parameter int KEEP_W = DATA_W / 8,
    parameter int STRB_W = DATA_W / 8,
    parameter int USER_W = 16,
    parameter int ID_W   = 8,
    parameter int DEST_W = 16,
    parameter int LAST_W = 1
) (
    input logic aclk,
    input logic aresetn
);
  logic [DATA_W-1:0] tdata;
  logic [ID_W-1:0] tid;
  logic [USER_W-1:0] tuser;
  logic [DEST_W-1:0] tdest;
  logic [LAST_W-1:0] tlast;
  logic [KEEP_W-1:0] tkeep;
  logic [STRB_W-1:0] tstrb;

  logic tvalid, tready;

  modport master(
      input aclk, aresetn,
      output tdata, tvalid, tkeep, tstrb, tlast, tid, tuser, tdest,
      input tready
  );

  modport slave(
      input aclk, aresetn,
      input tdata, tvalid, tkeep, tstrb, tlast, tid, tuser, tdest,
      output tready
  );

  modport monitor(
      input aclk, aresetn,
      input tdata, tvalid, tkeep, tstrb, tlast, tid, tuser, tdest, tready
  );

endinterface : AXIS_IF

Declarations/instances of interfaces of this type are declared as [N-1:0], since everything works fine when N=4.

[paul-demo]

Below is a complete example that demonstrates the bug in Verilator 5.024.

By continuously deleting things and linting the module using Verilator, I found that the problem is that Verilator doesn't handle names correctly when two different instances of an interface have the same local variable name in different layers of hierarchy in the design!

Basically a::myif and b::myif are two separate instances of the MYIF interface. Verilator thinks they're the same thing, and due to a::myif being a slave port, the assignment to b::myif is invalid according to Verilator, even though it appears to be valid Verilog and those are two distinct instances of the interface.

Ignore the fact that this doesn't appear to be a useful design. In my real design where I discovered this issue, there is no name collision when a::myif[3:0] is passed down the hierarchy to b::myif (basically in a generate loop where 4 copies of b are instantiated). But, when I changed from 4 to 1 instance of b, then a::myif[0:0] is passed down to just one instance of b::myif, and then the names collide and it reveals this bug in the way Verilator handles names across hierarchy.

interface MYIF ();
  logic tvalid;
  modport master(output tvalid);
  modport slave(input tvalid);
endinterface : MYIF

module tb ();
  MYIF myif ();
  a a_i (
    .myif
  );
endmodule

module a (
    MYIF.slave myif
);
  b b_i ();
endmodule

module b ();
  // The following produces an error:
  //  Attempt to drive input-only modport: 'tvalid'
  //  xx |   assign myif.tvalid = '0;
  //     |               ^~~~~~
  // https://github.com/verilator/verilator/issues/5120

  MYIF myif ();
  assign myif.tvalid = '0;


  // Renaming myif in this module to anything else makes the error go away.
  // This shows there is a name-overloading bug: Verilator is getting confused
  // by the fact that a::myif and b::myif have the same name. However, module b
  // has no ports, so a::myif cannot be passed down or otherwise collide with
  // b::myif -- since the module b has no ports there is no way that the submodule
  // b should have any effect on anything in module a.

  // Comment out the two lines above in this module, and uncomment these lines,
  // and the error goes away!
  // MYIF myif2 ();
  // assign myif2.tvalid = '0;

endmodule

[paul-demo]

This seems like a pretty major flaw -- it means Verilator could be mixing up SV interfaces across levels of design hierarchy, just by chance if they happen to have the same name in their local block of code.

[wsnyder]

This is because the module's parent has a interface of the same name, and the child and parent are both inlined.

As a workaround add to the child module

// verilator no_inline_module

Need more time to fix this, as simple fixes all broke other things. Interfaces are complicated.

[paul-demo]

No worries. I just avoid name reuse in the hierarchy for now when they don't actually refer to the same thing.