Option to Disable Randomization Logic

[seldridge]

Currently, there are a number of personal annoyances with how Chisel/FIRRTL emits verbose randomization logic. This issues is a proposal to add an option to disable randomization logic generation (which would be realized in FIRRTL). Additionally, this brings up a broader discussion about the motivation/need for randomization which I wrote up here.

Annoyances with randomization:

1. Randomization clutters up the Verilog for teaching/presentation/marketing purposes and there's no way to turn this off
2. (Tenuous, position:) Simulators support X-initialization. With this support, isn't randomization a parameter that a testing library should be passing to it's backend and not a Verilog generation problem?
3. (Unresearched position:) Verification teams know what they're doing (randomization can be turned off for them) and they would benefit from smaller Verilog files
4. Randomization logic is emitted even when registers are reset

Counterpoints supporting randomization:

1. Existing Verilog generation with randomization logic provides stability across simulators
2. chisel3.util contains generators that suffer from X-pessimism meaning that Chisel designs using the standard library may fail to simulate, but be perfectly correct.

At bare minimum, I'd like to have an option to disable randomization logic from being emitted. This shouldn't be the default, just something that a user can turn on if they have some good reason to. Two examples here would be: (1) me writing a Chisel snippet to go on the website and (2) ChiselTest opting to do randomization directly via a Verilator arg.

As further points of discussion:

1. What is the motivation in choosing to randomize in generated Verilog over relying on the simulator (VCS, Verilator, etc.) to do it? Are there deficiencies or inconsistencies across different simulators?
2. If a register is reset, why should it have randomization logic emitted? (Is it problematic to have this be X up until it's reset?)
3. Are there any Verification engineers out there who have an opinion on this? Do you rely on randomization logic that Chisel emits?
4. Generator writers should know if a generator artifact needs randomization to work or not. Would it benefit from introducing an annotation that can be used to selectively enable/disable randomization for specific registers?

As a concrete example, consider the following simple Chisel circuit. Assume that I'm somebody who's showing a colleague how to use Chisel:

class Foo extends MultiIOModule {
  val in = IO(Input(Bool()))
  val out = IO(Output(Bool()))
  out := RegNext(~in, init=0.U)
}

This produces the sane high FIRRTL IR:

circuit Foo :
  module Foo :
    input clock : Clock
    input reset : UInt<1>
    input in : UInt<1>
    output out : UInt<1>

    node _T = not(in)
    reg _T_1 : UInt<1>, clock with :
      reset => (reset, UInt<1>("h0"))
    _T_1 <= _T
    out <= _T_1

And vomits the following Verilog:

module Foo(
  input   clock,
  input   reset,
  input   in,
  output  out
);
  wire  _T;
  reg  _T_1;
  reg [31:0] _RAND_0;
  assign _T = ~in;
  assign out = _T_1;
`ifdef RANDOMIZE_GARBAGE_ASSIGN
`define RANDOMIZE
`endif
`ifdef RANDOMIZE_INVALID_ASSIGN
`define RANDOMIZE
`endif
`ifdef RANDOMIZE_REG_INIT
`define RANDOMIZE
`endif
`ifdef RANDOMIZE_MEM_INIT
`define RANDOMIZE
`endif
`ifndef RANDOM
`define RANDOM $random
`endif
`ifdef RANDOMIZE_MEM_INIT
  integer initvar;
`endif
`ifndef SYNTHESIS
initial begin
  `ifdef RANDOMIZE
    `ifdef INIT_RANDOM
      `INIT_RANDOM
    `endif
    `ifndef VERILATOR
      `ifdef RANDOMIZE_DELAY
        #`RANDOMIZE_DELAY begin end
      `else
        #0.002 begin end
      `endif
    `endif
  `ifdef RANDOMIZE_REG_INIT
  _RAND_0 = {1{`RANDOM}};
  _T_1 = _RAND_0[0:0];
  `endif // RANDOMIZE_REG_INIT
  `endif // RANDOMIZE
end // initial
`endif // SYNTHESIS
  always @(posedge clock) begin
    if (reset) begin
      _T_1 <= 1'h0;
    end else begin
      _T_1 <= _T;
    end
  end
endmodule

I would like to have an option to get something cleaner:

module Foo(
  input   clock,
  input   reset,
  input   in,
  output  out
);
  wire  _T;
  reg  _T_1;
  assign _T = ~in;
  assign out = _T_1;
  always @(posedge clock) begin
    if (reset) begin
      _T_1 <= 1'h0;
    end else begin
      _T_1 <= _T;
    end
  end
endmodule

Type of issue: feature request

Impact: API addition (no impact on existing code) | Verilog generation

Development Phase: request

Other information

None.

What is the use case for changing the behavior?

Discussed above.

[mwachs5]

For point "If a register is reset, why should it have randomization logic emitted? (Is it problematic to have this be X up until it's reset?)"

I agree -- I wasn't even aware this was the case.

[albert-magyar]

I think the concern is that it will leak X values into non-reset registers and increase the exposure of the simulation to X pessimism and optimism.

It's hard to say whether that would be a meaningful problem given our current strategy of emitting very conservative conditional logic and expressions. However, dropping initializations of reset registers would definitely make it harder to introduce some "better Verilog" patterns like streamlined procedural ifs/elses (which is a goal) without introducing cases where X optimism is observable.

I think this gets to the heart of a lot of our Verilog emission issues: we have multiple layers of safeguards against simulation issues, which are quite often excessively conservative. However, small peephole optimizations often leave us surprised by unexpected outcomes when suddenly some holes in the assumptions align. I think this is a good argument for a clean-sheet look at Verilog emission down the road.

However, for now, I think that making it optional is a win-win.

[aswaterman]

Removing randomization from registers that are reset will cause problems in general. For synchronous reset, running the clock to perform the reset will transfer Xs from registers that are reset into registers that are not reset. Even for async reset, some parts of the design may be reset at different times, so Xs lingering in not-yet-reset parts can pessimize other parts of the design.

My experience relying on simulators to do the randomization has been mixed. It’s obviously fine if the simulator has a 2-state-simulation option. My experience using VCS 4-state simulation + using VCS to randomize the initial state was quite negative; the feature was basically broken.

Having an option to suppress emission of randomization code seems fine; jettisoning randomization altogether is not gonna work out for us, though. But, separately, it would be good to improve the way the emitted code looks for marketing purposes.

On Thu, May 14, 2020 at 5:30 PM Albert Magyar notifications@github.com wrote:

I think the concern is that it will leak X values into non-reset registers and increase the exposure of the simulation to X pessimism and optimism.

It's hard to say whether that would be a meaningful problem given our current strategy of emitting very conservative conditional logic and expressions. However, dropping initializations of reset registers would definitely make it harder to introduce some "better Verilog" patterns like streamlined procedural ifs/elses (which is a goal) without introducing cases where X optimism is observable.

I think this gets to the heart of a lot of our Verilog emission issues: we have multiple layers of safeguards against simulation issues, which are quite often excessively conservative. However, small peephole optimizations often leave us surprised by unexpected outcomes when suddenly some holes in the assumptions align. I think this is a good argument for a clean-sheet look at Verilog emission down the road.

However, for now, I think that making it optional is a win-win.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/1444#issuecomment-628956356, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH3XQXDN5OZZ7UHUFLJZRLRRSEKDANCNFSM4NBDVAGQ .

[azidar]

An option to disable that randomization emission seems sane. Let's do it.