firrtl icon indicating copy to clipboard operation
firrtl copied to clipboard

RemoveResets Doesn't Look Through Nodes

Open seldridge opened this issue 3 years ago • 0 comments

The current implementation of RemoveResets will remove resets from registers which have are connected to invalid values within module scope looking through connects. This doesn't look through nodes, however.

Consider this first circuit:

circuit Foo:
  module Foo:
    input clock: Clock
    input reset: UInt<1>
    input d: UInt<8>
    output q: UInt<8>

    wire inv: UInt<8>
    inv is invalid

    wire tmp: UInt<8>
    tmp <= inv

    reg r: UInt<8>, clock with : (reset => (reset, tmp))
    r <= d
    q <= r

The register r is emitted as:

  always @(posedge clock) begin
    r <= d;
  end

Now consider this second circuit which replaces wire tmp with node tmp:

circuit Bar:
  module Bar:
    input clock: Clock
    input reset: UInt<1>
    input d: UInt<8>
    output q: UInt<8>

    wire inv: UInt<8>
    inv is invalid

    node tmp = inv

    reg r: UInt<8>, clock with : (reset => (reset, tmp))
    r <= d
    q <= r

This produces the following Verilog:

  always @(posedge clock) begin
    if (reset) begin
      r <= 8'h0;
    end else begin
      r <= d;
    end
  end

This issues is filed just for tracking purposes (and CIRCT will now make the same non-optimization in the node case). This behavior doesn't seem correct, but removing a reset could have surprising consequences.

seldridge avatar Dec 07 '21 03:12 seldridge