[FIRRTL] InferResets FullReset (formerly FART) errors due to "different" reset domains when the reset value is identical

[jackkoenig]

This is related to https://github.com/llvm/circt/issues/4886 in that this is sort of a special case. Fixing #4886 would fix this issue so long as infer-resets is followed by dedup (as it currently is)

Consider the following:

FIRRTL version 4.0.0
; CHECK-LABEL: module test(
circuit top :%[[
  {
    "class":"circt.FullResetAnnotation",
    "target":"~top|parent_1>reset",
    "resetType":"async"
  },
  {
    "class":"circt.FullResetAnnotation",
    "target":"~top|parent_2>reset",
    "resetType":"async"
  }
]]
  module child :
    input clock : Clock
    input in : UInt<8>
    output out : UInt<8>

    reg r : UInt<8>, clock
    connect r, in
    connect out, r

  module parent_1 :
    input clock : Clock
    input reset : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    inst c of child
    connect c.clock, clock
    connect c.in, in
    connect out, c.out

  module parent_2 :
    input clock : Clock
    input reset : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    inst c of child
    connect c.clock, clock
    connect c.in, in
    connect out, c.out

  public module top :
    input clock : Clock
    input reset1 : AsyncReset
    input reset2 : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    inst p1 of parent_1
    connect p1.clock, clock
    connect p1.reset, reset1
    connect p1.in, in

    inst p2 of parent_2
    connect p2.clock, clock
    connect p2.reset, reset2
    connect p2.in, in

    connect out, and(p1.out, p2.out)

We have 2 full reset domains, but they are identical (same port name and type). This currently errors with:

test.fir:15:3: error: module 'child' instantiated in different reset domains
  module child :
  ^
test.fir:15:3: note: see current operation: 
"firrtl.module"() ({
^bb0(%arg0: !firrtl.clock, %arg1: !firrtl.uint<8>, %arg2: !firrtl.uint<8>):
  %0 = "firrtl.reg"(%arg0) {annotations = [], name = "r", nameKind = #firrtl<name_kind droppable_name>} : (!firrtl.clock) -> !firrtl.uint<8>                                                                       
  "firrtl.matchingconnect"(%0, %arg1) : (!firrtl.uint<8>, !firrtl.uint<8>) -> ()
  "firrtl.matchingconnect"(%arg2, %0) : (!firrtl.uint<8>, !firrtl.uint<8>) -> ()
}) {annotations = [], convention = #firrtl<convention internal>, layers = [], portAnnotations = [[], [], []], portDirections = array<i1: false, false, true>, portLocations = [loc("test.fir":16:11), loc("test.fir":17:11), loc("test.fir":18:12)], portNames = ["clock", "in", "out"], portSyms = [], portTypes = [!firrtl.clock, !firrtl.uint<8>, !firrtl.uint<8>], sym_name = "child", sym_visibility = "private"} : () -> ()      
test.fir:30:5: note: instance 'p1/c' is in reset domain rooted at 'reset' of module 'parent_1'
    inst c of child
    ^
test.fir:26:11: note: reset domain 'reset' of module 'parent_1' declared here:
    input reset : AsyncReset
          ^
test.fir:41:5: note: instance 'p2/c' is in reset domain rooted at 'reset' of module 'parent_2'
    inst c of child
    ^
test.fir:37:11: note: reset domain 'reset' of module 'parent_2' declared here:
    input reset : AsyncReset
          ^

But this doesn't need to error, it could notice that the change required by each domain is the same.

[seldridge]

As written, the circuit is illegal because it is saying that the register should be connected to two different ports. I don't think that duplicating to resolve this is correct.

There's a path that could fix this with the following changes:

1. The annotation would need to be converted to an intrinsic statement.
2. The Full Reset part of InferResets would need to be broken out into a pass that runs after DedupModules.

FIRRTL version 4.0.0
; CHECK-LABEL: module test(
circuit top :
  module child :
    input clock : Clock
    input in : UInt<8>
    output out : UInt<8>

    reg r : UInt<8>, clock
    connect r, in
    connect out, r

  module parent_1 :
    input clock : Clock
    input reset : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    intrinsic(<circt.fullreset>, reset)

    inst c of child
    connect c.clock, clock
    connect c.in, in
    connect out, c.out

  module parent_2 :
    input clock : Clock
    input reset : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    intrinsic(<circt.fullreset>, reset)

    inst c of child
    connect c.clock, clock
    connect c.in, in
    connect out, c.out

  public module top :
    input clock : Clock
    input reset1 : AsyncReset
    input reset2 : AsyncReset
    input in : UInt<8>
    output out : UInt<8>

    inst p1 of parent_1
    connect p1.clock, clock
    connect p1.reset, reset1
    connect p1.in, in

    inst p2 of parent_2
    connect p2.clock, clock
    connect p2.reset, reset2
    connect p2.in, in

    connect out, and(p1.out, p2.out)