YosysHQ / yosys

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

Latch inferred for x signal #4325

Open spth opened 2 months ago

spth commented 2 months ago

Version

Yosys 0.38 (git sha1 543faed9c8c, gcc 13.2.0-13 -fPIC -Os)

On which OS did this happen?

Linux

Reproduction Steps

E.G. yosys -p "read_verilog -sv ../../icebreaker2_nosv.v; synth_ice40 -top icebreaker -json icebreaker2_synth.json" (but I get the same error also with -noflatten, or with the gatemate synthesis target). icebreaker2_nosv.v.gz test.vmem.gz

Expected Behavior

Signal result8 is implemented without latches

Actual Behavior

ERROR: Latch inferred for signal \cpu.\sv2v_autoblock_2.sv2v_autoblock_3.result8' from always_comb process\cpu.$proc$../../icebreaker2_nosv.v:0$1774'.

povik commented 2 months ago

Can you run the following experiment?

Take logic [7:0] result8; result8 = 1'sbx; from line 778 of icebreaker2_nosv.v, and put it at the top process scope somewhere above line 705. Also rename the other result8 signals on lines 911 and 1100 so there's no name conflict. I expect the error will go away. Please confirm.

We have an issue but this will help confirm what it is.

spth commented 2 months ago

Yes, moving the signal to line 705 is a workaround (I renamed this signal instead of the other one), which makes yosys proceed a few lines further:

No latch inferred for signal `\cpu.\sv2v_autoblock_2.result8A' from process `\cpu.$proc$../../icebreaker2_nosv.v:0$1774'.
…
ERROR: Latch inferred for signal `\cpu.\sv2v_autoblock_2.sv2v_autoblock_3.sv2v_autoblock_4.addsub_result' from always_comb process `\cpu.$proc$../../icebreaker2_nosv.v:0$1774'.

I.e. into this part of the source:

if (opcode_is_sub(opcode) || opcode_is_cp(opcode)) begin : sv2v_autoblock_4
                    logic [20:0] addsub_result;
                    if (!swapop)
                        addsub_result = addsub({8'h00, acc8}, {8'h00, ~op8}, 1, 0);
                    else
                        addsub_result = addsub({8'h00, ~acc8}, {8'h00, op8}, 1, 0);
                    next_f = {3'b000, addsub_result[4], addsub_result[3], addsub_result[2], addsub_result[1], addsub_result[0]};
                    result8A = addsub_result[12:5];
                end
povik commented 2 months ago

@spth Thanks!

Leaving a note to the community at large: I wrote a proc_usage pass in yosys-slang to address exactly this: https://github.com/povik/yosys-slang/blob/master/proc_usage.cc

    proc_usage [selection]

This pass identifies assignments in processes which are unused outside of the
process of origin. As such, those assignments can be removed as a driver of
a module-level signal. It is important to remove those assignments ahead of the
`proc_dlatch` pass to not to infer spurious latches, which are a compilation
failure for `always_comb` blocks.

This pass can be combined with the in-tree Verilog frontend, I tested read_verilog -sv icebreaker2_nosv.v; proc_clean; proc_rmdead; proc_prune; proc_init; proc_arst; proc_rom; proc_mux; proc_clean; proc_usage; proc_dlatch; proc_dff; proc_clean; opt_expr -keepdc on the design in question here, and it passes.

povik commented 2 months ago

Not sure if this is a bug or a feature request, it's a SystemVerilog extension but the README claims we do support always_comb and other synthesis tools accept this.