[PrepareForEmission] Temporaries are not spilled in a procedural region

[uenoku]

The following IR is derived from a test case of https://github.com/llvm/circt/pull/3116 which workarounded verilator hangs.

hw.module @Verilator3405() -> () {
    %state = sv.reg : !hw.inout<i2>
    sv.initial {
      %state_read = sv.read_inout %state : !hw.inout<i2> 
      %read = comb.extract %state_read from 0 : (i2) -> i1
      %lhs = comb.or %read, %read, %read, %read, %read, %read, %read, %read, %read, %read, %read, %read : i1
      %rhs = comb.or %read, %read, %read, %read, %read, %read, %read, %read, %read, %read, %read, %read  : i1
      %out = comb.concat %lhs, %rhs : i1, i1
      sv.passign %state, %out : i2
    }
  }

With circt-opt -export-verilog -lowering-options=disallowLocalVariables, we get an expected result:

module Verilator3405(); 
  wire       _GEN; 
  wire       _GEN_0;
  reg  [1:0] state;  

  assign _GEN_0 = state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] |
                state[0] | state[0] | state[0] | state[0];
  assign _GEN = state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] |
                state[0] | state[0] | state[0] | state[0]; 
  initial       
    state <= {_GEN_0, _GEN};
endmodule

%lhs and %rhs are spilled to temporary wires. On the other hand with circt-opt -export-verilog, the result is:

module Verilator3405();
  reg [1:0] state; 

  initial 
    state <= {state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] |
                                state[0] | state[0] | state[0] | state[0], state[0] | state[0] | state[0] | state[0] |
                                state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0]}; 
endmodule

I think it should be:

  initial begin
    automatic logic _GEN_0 = state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] |
                state[0] | state[0] | state[0] | state[0];
    automatic logic _GEN = state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] | state[0] |
                state[0] | state[0] | state[0] | state[0]; 
       
    state <= {_GEN_0, _GEN};
 end

[mikeurbach]

So the issue here is not necessarily a logical error, but that the spill can't (and doesn't) occur?

I believe that stopped happening even earlier than #3116, when the large expression spilling was removed from ExportVerilog in https://github.com/llvm/circt/commit/586f3dc63dc39fcb8d0ec27667af00cc65a4f332.

We're relying on Prepare to spill what needs to be spilled, but Prepare is an IR transform, and there is no automatic logic IR. ExportVerilog was free to print automatic logic before, but we are removing all spilling from ExportVerilog. Is that the root of the issue?

If I understand this correctly, I think your point on Discourse is the right one, we should declare something like an sv.automatic_logic op, which Prepare can use to restore the ability to spill in procedural regions.

[uenoku]

We're relying on Prepare to spill what needs to be spilled, but Prepare is an IR transform, and there is no automatic logic IR. ExportVerilog was free to print automatic logic before, but we are removing all spilling from ExportVerilog. Is that the root of the issue?

Yes, exactly!

If I understand this correctly, I think your point on Discourse is the right one, we should declare something like an sv.automatic_logic op, which Prepare can use to restore the ability to spill in procedural regions.

I think sv.automatic_logic makes sense to me. Another idea issv.internal.temporary op which is lowered to wires if op is in non-procedural regions, or lowered to automatic logic in procedural regions.