[ExportVerilog] Don't generate automatic variable if the tool doesn't support it

[lattner]

According to this thread, Yosys doesn't support automatic logic values in an always block. We generate these when spilling into new declarations due to overly long line length, and when things are multiply used in always blocks. Here's an example from the thread:

always @(posedge clock) begin	// Monitor.scala:397:15
    automatic logic _T_12;	// Monitor.scala:396:20
    automatic logic _T_13;	// Monitor.scala:549:20

    _T_12 = _T_2 & ~(|a_first_counter);	// Edges.scala:229:28, :230:25, Monitor.scala:396:20
    _T_13 = _T_3 & ~(|d_first_counter);	// Edges.scala:229:28, :230:25, Monitor.scala:549:20
    opcode <= _T_12 ? io_in_a_bits_opcode : opcode;	// Monitor.scala:390:32, :397:15
    param <= _T_12 ? io_in_a_bits_param : param;	// Monitor.scala:391:32, :398:15
    size <= _T_12 ? io_in_a_bits_size : size;	// Monitor.scala:392:32, :399:15
    source <= _T_12 ? io_in_a_bits_source : source;	// Monitor.scala:393:32, :400:15
    address <= _T_12 ? io_in_a_bits_address : address;	// Monitor.scala:394:32, :401:15
    opcode_1 <= _T_13 ? io_in_d_bits_opcode : opcode_1;	// Monitor.scala:542:29, :550:15
    param_1 <= _T_13 ? io_in_d_bits_param : param_1;	// Monitor.scala:543:29, :551:15
    size_1 <= _T_13 ? io_in_d_bits_size : size_1;	// Monitor.scala:544:29, :552:15
    source_1 <= _T_13 ? io_in_d_bits_source : source_1;	// Monitor.scala:545:29, :553:15
    sink <= _T_13 ? io_in_d_bits_sink : sink;	// Monitor.scala:546:29, :554:15
    denied <= _T_13 ? io_in_d_bits_denied : denied;	// Monitor.scala:547:29, :555:15
  end // always @(posedge)

It is suggested that the verilog printer inject top level declarations like so:

logic _T_12;	// Monitor.scala:396:20
logic _T_13;	// Monitor.scala:549:20
always @(posedge clock) begin	// Monitor.scala:397:15
    _T_12 = _T_2 & ~(|a_first_counter);	// Edges.scala:229:28, :230:25, Monitor.scala:396:20
    _T_13 = _T_3 & ~(|d_first_counter);	// Edges.scala:229:28, :230:25, Monitor.scala:549:20
    opcode <= _T_12 ? io_in_a_bits_opcode : opcode;	// Monitor.scala:390:32, :397:15
    param <= _T_12 ? io_in_a_bits_param : param;	// Monitor.scala:391:32, :398:15
    size <= _T_12 ? io_in_a_bits_size : size;	// Monitor.scala:392:32, :399:15
    source <= _T_12 ? io_in_a_bits_source : source;	// Monitor.scala:393:32, :400:15
    address <= _T_12 ? io_in_a_bits_address : address;	// Monitor.scala:394:32, :401:15
    opcode_1 <= _T_13 ? io_in_d_bits_opcode : opcode_1;	// Monitor.scala:542:29, :550:15
    param_1 <= _T_13 ? io_in_d_bits_param : param_1;	// Monitor.scala:543:29, :551:15
    size_1 <= _T_13 ? io_in_d_bits_size : size_1;	// Monitor.scala:544:29, :552:15
    source_1 <= _T_13 ? io_in_d_bits_source : source_1;	// Monitor.scala:545:29, :553:15
    sink <= _T_13 ? io_in_d_bits_sink : sink;	// Monitor.scala:546:29, :554:15
    denied <= _T_13 ? io_in_d_bits_denied : denied;	// Monitor.scala:547:29, :555:15
  end // always @(posedge)

This should only be done under a LoweringOptions flag.

[WonHoYoo]

IMO, it would be much more hardware engineer-friendly verilog emitter if it can place all wire, reg, logic varlable at the start of the module. :)

[lattner]

Sure, people will be able to pass the flag to get that behavior if they want.

[seldridge]

I think the above spilling outside the always block will result in flip flops being created for _T_12 and _T_13. To prevent this it may need to spill the assignment to _T_12 and _T_13. Something like:

wire _T_12 = _T_2 & ~(|a_first_counter);
wire _T_13 = _T_3 & ~(|d_first_counter);
always @(posedge clock) begin
  /* ... */
end

@drom: thoughts about above?

The mixing of blocking and non-blocking assignments inside the always @(posedge ...) is also frowned upon. Tools should do the right thing with that, but I have no idea if they will. It would be better to get all the continuous assignment stuff into initialization (as above), into assign, or into an always_comb/always @* block. This code motion is getting difficult to do inside the export verilog pre-pass and it was proposed today to spin this out to a separate pass so that other passes can be inserted between it to enforce more restrictive Verilog semantics. (I.e., automatic logic could be generated in one pass and then hoisted out in another.)

Basically, there's no great solution here because automatic logic or var appears to be Verilog's only way to define a temporary and the tool support is empirically spotty. 😭

Anything that you try here, you may want to run through both of:

verilator --lint-only -language 1800-2017 -Wall Foo.v
verilator --lint-only -lanugage 1364-2001 -Wall Foo.v

This will give you some feedback on whether or not the output makes sense. 😄

[drom]

reg assigned inside always @(posedge clock ...) block is flip-flop.
reg assigned inside always @* is combinatorial logic

[lattner]

I can't find any way to support this in a reasonable way: icarus and yosys don't support assigning to wires declared at top level in an always block. Neither supports logic declarations either (this is an SV thing).

[lattner]

I believe I can just spill all of the expressions to the top level as wire decls. That is gross, but will be functional. I'll take care of this.

[lattner]

This is actually really nasty, we need to emit reg's for top level things that have side effects, e.g. `RANDOM. This is what SFC is generating:

`ifdef RANDOMIZE_REG_INIT
  reg [31:0] _RAND_0;
`endif // RANDOMIZE_REG_INIT
  reg  _tmp2;

  initial begin
    `ifdef RANDOMIZE_REG_INIT
      _RAND_0 = {1{`RANDOM}};
      _tmp2 = _RAND_0[0:0];
`endif // RANDOMIZE_REG_INIT
end // initial

[lattner]

Sounds like the heuristic is:

1. hoist as much to the top level as wires as possible if it cannot be emitted inline.
2. emit as regs if it can't be hoisted.

[lattner]

Part #2 is handled by 3c8b4b4.

[lattner]

Part #1 is implemented with 05c7a13.