[FIRRTL] FSRT: not finding all no-reset registers

[youngar]

FIRRTL version 4.0.0
circuit Foo: %[[
  {
   "class": "circt.FullResetAnnotation",
   "target": "~Foo|Foo>r",
   "resetType": "sync"
  }
]]
  public module Foo:
    input c : Clock
    input i : UInt<8>
    input r : UInt<1>
    output o : UInt<8>
    
    wire w : UInt<8>
    reg reg : UInt<8>, c with:
      reset => (r, w)
    connect w, reg
    connect reg, i
    connect o, reg

gives:

module Foo(
  input        c,
  input  [7:0] i,
  input        r,
  output [7:0] o
);

  reg [7:0] reg_0;
  always @(posedge c) begin
    if (r)
      reg_0 <= reg_0;
    else
      reg_0 <= i;
  end // always @(posedge)
  `ifdef ENABLE_INITIAL_REG_
    `ifdef FIRRTL_BEFORE_INITIAL
      `FIRRTL_BEFORE_INITIAL
    `endif // FIRRTL_BEFORE_INITIAL
    initial begin
      automatic logic [31:0] _RANDOM[0:0];
      `ifdef INIT_RANDOM_PROLOG_
        `INIT_RANDOM_PROLOG_
      `endif // INIT_RANDOM_PROLOG_
      `ifdef RANDOMIZE_REG_INIT
        _RANDOM[/*Zero width*/ 1'b0] = `RANDOM;
        reg_0 = _RANDOM[/*Zero width*/ 1'b0][7:0];
      `endif // RANDOMIZE_REG_INIT
    end // initial
    `ifdef FIRRTL_AFTER_INITIAL
      `FIRRTL_AFTER_INITIAL
    `endif // FIRRTL_AFTER_INITIAL
  `endif // ENABLE_INITIAL_REG_
  assign o = reg_0;
endmodule

Besides the fact that we are missing a canonicalization, it is pretty clear here that the register does not have a reset, and so the FSRT should have added a reset to it.

[youngar]

I also want to point out that we do perform the canonicalization if we remove the intermediate wire:

FIRRTL version 4.0.0
circuit Foo: %[[
  {
   "class": "circt.FullResetAnnotation",
   "target": "~Foo|Foo>r",
   "resetType": "sync"
  }
]]
  public module Foo:
    input c : Clock
    input i : UInt<8>
    input r : UInt<1>
    output o : UInt<8>
    reg reg : UInt<8>, c with:
      reset => (r, reg)
    connect reg, i
    connect o, reg

to

  reg [7:0] reg_0;
  always @(posedge c) begin
    if (r)
      reg_0 <= 8'h0;
    else
      reg_0 <= i;
  end // always @(posedge)

I think we handle this right in the parser? And so FRT will see that this register does not have a reset and add one.

[youngar]

Here is another simpler example. The register has a reset but it is hard-wired to 0, and in the resulting verilog, the register has no reset.

FIRRTL version 4.0.0
circuit Foo: %[[
  {"class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation", "target":"~Foo|Foo>reset"}
]]
  public module Foo:
    input c : Clock
    input i : UInt<8>
    input reset : AsyncReset
    output o : UInt<8>

    reg r : UInt<8>, c with:
      reset => (asAsyncReset(UInt<1>(0)), UInt<8>(-1))
    connect r, i
    connect o, r

module Foo(
  input        c,
  input  [7:0] i,
  input        reset,
  output [7:0] o
);

  reg [7:0] r;
  always @(posedge c)
    r <= i;
  `ifdef ENABLE_INITIAL_REG_
    `ifdef FIRRTL_BEFORE_INITIAL
      `FIRRTL_BEFORE_INITIAL
    `endif // FIRRTL_BEFORE_INITIAL
    initial begin
      automatic logic [31:0] _RANDOM[0:0];
      `ifdef INIT_RANDOM_PROLOG_
        `INIT_RANDOM_PROLOG_
      `endif // INIT_RANDOM_PROLOG_
      `ifdef RANDOMIZE_REG_INIT
        _RANDOM[/*Zero width*/ 1'b0] = `RANDOM;
        r = _RANDOM[/*Zero width*/ 1'b0][7:0];
      `endif // RANDOMIZE_REG_INIT
    end // initial
    `ifdef FIRRTL_AFTER_INITIAL
      `FIRRTL_AFTER_INITIAL
    `endif // FIRRTL_AFTER_INITIAL
  `endif // ENABLE_INITIAL_REG_
  assign o = r;
endmodule