chipsalliance / firrtl

Flexible Intermediate Representation for RTL
https://www.chisel-lang.org/firrtl/
Apache License 2.0
719 stars 176 forks source link

FIRRTL should not inline expressions in sensitivity lists #1937

Open jackkoenig opened 3 years ago

jackkoenig commented 3 years ago

Checklist

What is the current behavior?

Given:

circuit test :
  module test :
    input clock : Clock
    input reset1 : AsyncReset
    input reset2 : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    node _T_1 = asAsyncReset(or(asUInt(reset1), asUInt(reset2)))
    reg r : UInt<8>, clock with : (reset => (_T_1, UInt(0)))
    r <= in
    out <= r

Running FIRRTL 1.4.0 gives:

module test(
  input        clock,
  input        reset1,
  input        reset2,
  input  [7:0] in,
  output [7:0] out
);
`ifdef RANDOMIZE_REG_INIT
  reg [31:0] _RAND_0;
`endif // RANDOMIZE_REG_INIT
  reg [7:0] r;
  assign out = r;
  always @(posedge clock or posedge reset1 | reset2) begin
    if (reset1 | reset2) begin
      r <= 8'h0;
    end else begin
      r <= in;
    end
  end
// ...
endmodule

Synthesis tools don't accept posedge reset1 | reset2

What is the expected behavior?

We should assign reset1 | reset2 to a wire and use that in the sensitivity list

Steps to Reproduce

With coursier installed as cs, you can just put the above FIRRTL in test.fir and run:

$ cs launch edu.berkeley.cs:firrtl_2.12:1.4.0 -M firrtl.stage.FirrtlMain -- -i test.fir -o out -X verilog

Your environment

External Information

jackkoenig commented 3 years ago

Known workaround to just dontTouch the AsyncReset expression which prevents it being inlined.