YosysHQ / yosys

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

Add support/workaround for flattening `inout` port with tri-state signals #4501

Open Rodrigodd opened 4 months ago

Rodrigodd commented 4 months ago

Feature Description

I am currently trying to magically speed up the simulation of dmgcpu by checking if I can get yosys to optimize out all uses of high-impedance signals in the design, write it to Verilog, and then simulate it using Verilator.

One problem I am stumbling on is that, after I flatten the design and write_verilog, signals don't propagate into modules through inout ports when they are not being internally driven.

For example, in the design below I have two submodules, each one driving the bus on one half of the cycle. I also have a bus1 signal that exposes the internal bus of the first module, for easier visualization.

module Top(bus, clk, bus1);
    inout [7:0] bus;
    input clk;
    output [7:0] bus1;

    Bottom1 b1(bus, clk, bus1);
    Bottom2 b2(bus, clk);
endmodule

module Bottom1(bus, clk, b1);
    inout [7:0] bus;
    input clk;
    output [7:0] b1;

    assign bus = clk ? 8'haa : 8'bz;
    assign b1 = bus;

endmodule

module Bottom2(bus, clk);
    inout [7:0] bus;
    input clk;
    assign bus = ~clk ? 1'h0f : 8'bz;
endmodule

The entire project, including a Makefile and the run.v testbench: test.zip

If I simulate it with iverilog -o test.vvp test.v run.v -DVCDFILE="\"test.vcd\"" && vvp test.vvp -vcd, the bus and bus1 signals are equal as expected:

image

But if I flatten the design first (yosys -p "read_verilog test.v; hierarchy -check; flatten; write_verilog test.yosys.v;"), the bus1 is no longer equal to bus, displaying a high-impedance state:

image

Expected Solution

The problem is that, when flattening, inout ports are treated like output ports, directionally connecting the internal wire with the external one. We can see that in the RTLIL format (yosys -p "read_verilog test.v; hierarchy -check; flatten; write_rtlil test.yosys.rtlil;"):

# Generated by Yosys 0.42 (git sha1 9b6afcf3f, g++ 14.1.1 -march=x86-64 -mtune=generic -O2 -fno-plt -fexceptions -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffile-prefix-map=/build/yosys/src=/usr/src/debug/yosys -fPIC -Os)
autoidx 5
module \Top
  # ... omitted for brevity ...
  connect \b1.bus $flatten\b1.$ternary$test.v:15$1_Y
  connect \b1.b1 \b1.bus
  connect \bus \b1.bus # internal signal `b1.bus` -> external signal `bus` # (this should be bi-directional)
  connect \b1.clk \clk # external signal `clk`    -> internal signal `b1.clk`
  connect \bus1 \b1.b1
  connect \b2.bus $flatten\b2.$ternary$test.v:23$4_Y
  connect \bus \b2.bus
  connect \b2.clk \clk
end

A proper solution would make a bidirectional connection between the two wires and eventually replace both signals with a single wire (by eventually, I may mean immediately, because I am not sure if it is possible to add bidirectional connections to Yosys' internal representation). Another solution would be to add an optimization pass that replaces all connected wires with a single one (which would treat all ports as inout, but that is not a problem in my particular case).

If anyone is willing to give me a direction on how any of the two solutions above can be implemented, I will give it a shot.

whitequark commented 4 months ago

In general I would strongly advise against using inout ports for anything except connecting to I/O buffers. (In the HDL I maintain, Amaranth, the equivalent of inout wires is a distinct type that you can't directly use with logic.) While in principle supported in Verilog, their practical support is so sparse and full of bugs that it is not worth it chasing down all those bugs, as opposed to the alternative of using normal logic to perform multidirectional connections.

I realize that you are working with an existing design. As a solution to that I would suggest a pre-processing pass that converts internal inout wires to normal logic and then optimizing that logic.

Rodrigodd commented 4 months ago

In general I would strongly advise against using inout ports for anything except connecting to I/O buffers.

Yeah, I would also avoid using tri-state signals, but the project I mentioned is reverse-engineering the logic circuit from dieshots of the main SoC of a GameBoy Model B, and for maintainability it is important that the logic maps very closely to the actual circuit.

As a solution to that, I would suggest a pre-processing pass that converts internal inout wires to normal logic and then optimizes that logic.

Yes, that would be the optimization pass I mentioned in the last paragraph. Or is there already a pre-processing pass that does what I want to achieve?

whitequark commented 4 months ago

Or is there already a pre-processing pass that does what I want to achieve?

I don't think so. flatten in particular is... cursed, I had to rework it a fair amount a few years ago so that it would preserve source level names, and that didn't make me a happier person. If you would like compiler advice (general or specific to Yosys) I think you can reach out to me and I'll share it.

Rodrigodd commented 4 months ago

If you would like compiler advice (general or specific to Yosys), I think you can reach out to me and I'll share it.

That would be great! I didn't take a big look at the code base, but I remember seeing that each pre-processing pass is implemented in its own file and works independently from one another, mostly traversing and mutating the RTLIL representation (but I only took a look at the tribuf pass implementation).

If you could tell me which approach you think is better, either making flatten have better support or adding a pass for merging connected wires. I think the latter approach sounds more approachable, and I may need it anyway to make the tribuf pass work more consistently (I am hoping that the tribuf pass will optimize out most uses of high-impedance signals).

And if you could also point to any previous pass I can base my implementation on, or if there are any resources about creating a new pass. But if there are no resources specific to that, no worries, I can probably figure things out by myself. Thanks.

whitequark commented 4 months ago

I would advise you to leave flatten as it is and write a custom pass. Feel free to reach out to me on IRC or Matrix for advice (whitequark on libera, @whitequark:matrix.org)

Rodrigodd commented 1 month ago

An update on this issue. I got to the point of implementing a pass that does what I described above, replacing all wires in the same net with a single representative wire. You can see the implementation here. However, after running some small-scale tests with opt_clean and reviewing the code, it appears that opt_clean was supposed to do exactly what I needed, although it wasn't working for the entire design.

Later on, I extended the tribuf pass to propagate all tri-state signals, which superseded this issue (see https://github.com/YosysHQ/yosys/pull/4661), so I didn't investigate this further.

I will close this issue as resolved when the linked PR is merged.