YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.37k stars 874 forks source link

Wider ports in SystemVerilog interfaces are treated as single bit wire #3592

Open Henkru opened 1 year ago

Henkru commented 1 year ago

Version

Yosys 0.24+10 (git sha1 69cbef966, clang 10.0.0-4ubuntu1 -fPIC -Os)

On which OS did this happen?

Linux

Reproduction Steps

When a SystemVerilog code contains an interface with wider fields than a single bit, it appears that the bus is being treated as a single bit wire rather than n bits. See the screenshot below.

  1. Create the following top.sv file.
    
    interface inf;
    logic [7:0] a;
    modport Slave(input a);
    endinterface

module top ( input clk, inf.Slave bus
);

logic [7:0] r;
initial r = '0;

always_ff @(posedge clk) begin
    r <= bus.a; 
end

always @(posedge clk) begin
    cover(r == 8'b0000_0001); // cover, since bus.a as treated as a one bit value
    cover(r == 8'b0000_0011); // not cover
end

endmodule

2. Create the following `top.sby` file

[options] mode cover

[engines] smtbmc

[script] read -sv -formal top.sv prep -top top

[files] top.sv

3. Execute `sby -f top.sby`
4. Examine the generated waveform `top/engine_0/trace0.vcd`. The `bus.a` is single wire as shown in the following screenshot.

![gtkwave](https://user-images.githubusercontent.com/4019880/208317967-4fa223c3-f85e-4818-9181-79f8598a22e3.png)

### Expected Behavior

Both cover statements should be reached.

### Actual Behavior

Only the first cover case is reached since the length of the `bus.a` is 1 and not 8. Yosys provide the following output which seems to be related to #1053.

SBY 22:27:03 [top] base: top.sv:15: Warning: Identifier `\bus.a' is implicitly declared. SBY 22:27:03 [top] base: Warning: Wire top.\bus.a is used but has no driver. SBY 22:27:03 [top] base: finished (returncode=0) SBY 22:27:03 [top] prep: starting process "cd top/model; yosys -ql design_prep.log design_prep.ys" SBY 22:27:03 [top] prep: Warning: Wire top.\bus.a is used but has no driver.


I really don't know the code base but based on the original interface PR, I assume the issue lives in this [area](https://github.com/YosysHQ/yosys/blob/6c65ca4e50cc6712d9293b9630afdf67af89ef61/frontends/ast/genrtlil.cc#L1226-L1254).
jix commented 1 year ago

I can only reproduce this issue when using read_verilog with the -defer option, which read does by default. Using read_verilog directly without that option works here and may be a suitable workaround for you.

The issue seems to be that deferred RTLIL generation for the top module doesn't trigger deferred RTLIL generation for the interface used by the top module which then results in the interface not being instantiated. The placeholder port for the modport is then being removed without actually adding the modport ports. This in turn ends up with an undeclared implicit 1-bit wire for bus.a (the corresponding warning is present even without -defer, a false positive, as the reference to bus.a is later correctly resolved by hierarchy, while with -defer the warning is printed again after interfaces are supposed to be resolved).

I don't know how to fix this yet, as I'm not too familiar with the implementation of hierarchy.

Henkru commented 1 year ago

Thanks for the clarification. I can confirm that read_verilog -sv top.sv works as a workaround.