CXXRTL: internal assertion fires as of commit efc43270

[Wren6991]

As requested by @whitequark, opening a second issue for the other assertion mentioned on #2881

Steps to reproduce the issue

Either this bugpoint output hazard3-shifter-bug.il:

# Generated by Yosys 0.9+4081 (git sha1 efc43270f, clang 10.0.0-4ubuntu1 -fPIC -Os)
autoidx 92
attribute \src "hazard3_shift_barrel.v:1.1-38.10"
attribute \cells_not_processed 1
attribute \dynports 1
module \hazard3_shift_barrel
  parameter \W_DATA 32
  parameter \W_SHAMT 5
  attribute \src "hazard3_shift_barrel.v:18.1-21.4"
  wire width 32 $0\i[31:0]
  attribute \src "hazard3_shift_barrel.v:23.1-31.4"
  wire width 32 $1\i[31:0]
  wire $delete_wire$87
  wire $delete_wire$88
  wire $delete_wire$89
  wire $delete_wire$90
  wire $delete_wire$91
  attribute \src "hazard3_shift_barrel.v:12.9-12.10"
  wire width 32 signed \i
  attribute \src "hazard3_shift_barrel.v:18.1-21.4"
  process $proc$hazard3_shift_barrel.v:18$5
    sync always
      update \i $0\i[31:0]
  end
  attribute \src "hazard3_shift_barrel.v:23.1-31.4"
  process $proc$hazard3_shift_barrel.v:23$38
    attribute \src "hazard3_shift_barrel.v:26.3-29.6"
    switch $delete_wire$87
      case 1'1
      case 
    end
    attribute \src "hazard3_shift_barrel.v:26.3-29.6"
    switch $delete_wire$88
      case 1'1
      case 
    end
    attribute \src "hazard3_shift_barrel.v:26.3-29.6"
    switch $delete_wire$89
      case 1'1
      case 
    end
    attribute \src "hazard3_shift_barrel.v:26.3-29.6"
    switch $delete_wire$90
      case 1'1
      case 
    end
    attribute \src "hazard3_shift_barrel.v:26.3-29.6"
    switch $delete_wire$91
      case 1'1
      case 
    end
    sync always
      update \i $1\i[31:0]
  end
end

Or this Verilog source hazard3_shift_barrel.v, from which the above ilang was reduced:

module hazard3_shift_barrel #(
	parameter W_DATA = 32,
	parameter W_SHAMT = 5
) (
	input wire [W_DATA-1:0]  din,
	input wire [W_SHAMT-1:0] shamt,
	input wire               right_nleft,
	input wire               arith,
	output reg [W_DATA-1:0]  dout
);

integer i;

reg [W_DATA-1:0] din_rev;
reg [W_DATA-1:0] shift_accum;
wire sext = arith && din_rev[0]; // haha

always @ (*) begin
	for (i = 0; i < W_DATA; i = i + 1)
		din_rev[i] = right_nleft ? din[W_DATA - 1 - i] : din[i];
end

always @ (*) begin
	shift_accum = din_rev;
	for (i = 0; i < W_SHAMT; i = i + 1) begin
		if (shamt[i]) begin
			shift_accum = (shift_accum << (1 << i)) |
				({W_DATA{sext}} & ~({W_DATA{1'b1}} << (1 << i)));
		end
	end
end

always @ (*) begin
	for (i = 0; i < W_DATA; i = i + 1)
		dout[i] = right_nleft ? shift_accum[W_DATA - 1 - i] : shift_accum[i];
end

endmodule

Use any Yosys version from efc4327 to 9af8895. Either of these will trigger the assertion:

yosys -p "read_ilang hazard3-shifter-bug.il; write_cxxrtl dut.cpp"

or

yosys -p "read_verilog hazard3_shift_barrel.v; write_cxxrtl dut.cpp"

Expected behavior

Write out a cpp model, or complain about the RTL source

Actual behavior

ERROR: Assert `flow.wire_comb_defs[it].size() == 1' failed in backends/cxxrtl/cxxrtl_backend.cc:2811.

Further troubleshooting

If I check out efc4327 and re-add this line that was removed by that commit, the assertion does not fire on either the Verilog source or the reduced testcase:

diff --git a/backends/cxxrtl/cxxrtl_backend.cc b/backends/cxxrtl/cxxrtl_backend.cc
index dfdb9df74..cec5dc0a5 100644
--- a/backends/cxxrtl/cxxrtl_backend.cc
+++ b/backends/cxxrtl/cxxrtl_backend.cc
@@ -2794,6 +2794,7 @@ struct CxxrtlWorker {
                                for (auto wire : module->wires()) {
                                        const auto &wire_type = wire_types[wire];
                                        auto &debug_wire_type = debug_wire_types[wire];
+                                       if (wire_type.type == WireType::UNUSED) continue;
 
                                        if (!debug_info) continue;
                                        if (wire->port_input || wire_type.is_buffered())

I can also avoid tripping the assertion with a small change in the Verilog source, namely putting all 3 accumulator loops in a single always @ (*) block, instead of across 3 of them, which is probably a better way to write it anyway:

module hazard3_shift_barrel #(
	parameter W_DATA = 32,
	parameter W_SHAMT = 5
) (
	input wire [W_DATA-1:0]  din,
	input wire [W_SHAMT-1:0] shamt,
	input wire               right_nleft,
	input wire               arith,
	output reg [W_DATA-1:0]  dout
);

integer i;

reg [W_DATA-1:0] din_rev;
reg [W_DATA-1:0] shift_accum;
wire sext = arith && din_rev[0]; // haha

always @ (*) begin
	for (i = 0; i < W_DATA; i = i + 1)
		din_rev[i] = right_nleft ? din[W_DATA - 1 - i] : din[i];
	shift_accum = din_rev;

	for (i = 0; i < W_SHAMT; i = i + 1) begin
		if (shamt[i]) begin
			shift_accum = (shift_accum << (1 << i)) |
				({W_DATA{sext}} & ~({W_DATA{1'b1}} << (1 << i)));
		end
	end

	for (i = 0; i < W_DATA; i = i + 1)
		dout[i] = right_nleft ? shift_accum[W_DATA - 1 - i] : shift_accum[i];
end

endmodule

[whitequark]

Reduced further:

module \hazard3_shift_barrel
  wire width 32 $0\i[31:0]
  wire width 32 $1\i[31:0]
  wire width 32 signed \i
  process $proc$hazard3_shift_barrel.v:18$5
    sync always
      update \i $0\i[31:0]
  end
  process $proc$hazard3_shift_barrel.v:23$38
    sync always
      update \i $1\i[31:0]
  end
end

Sigh okay, this is... ultimately a problem with the Verilog frontend. It produces unsound RTLIL, where i is written from multiple processes. The CXXRTL crash points to a genuine problem handling certain edge cases and can be fixed on its own though.