[FIRRTL] FART: logic to reuse existing port can result in the wrong reset

[youngar]

Given this FIRRTL:

FIRRTL version 4.0.0
circuit Foo: %[[
  {"class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation", "target":"~Foo|Foo>r"}
]]
  public module Foo:
    input p : UInt<1>
    input r : AsyncReset
    input c : Clock
    inst bar of Bar
    connect bar.r, asAsyncReset(UInt<1>(0))
    connect bar.c, c

  module Bar:
    input c : Clock
    input r : AsyncReset

    reg reg : UInt<8>, c

When compiled with firtool with no additional options, we see InferResets uses the port Bar.r as the reset domain for the internals of Bar.

firrtl.circuit "Foo" {
 firrtl.module @Foo(in %p: !firrtl.uint<1>, in %r: !firrtl.asyncreset [{class = "sifive.enterprise.firrt
l.FullAsyncResetAnnotation"}], in %c: !firrtl.clock) attributes {convention = #firrtl<convention scalariz
ed>} {
   %bar_c, %bar_r = firrtl.instance bar @Bar(in c: !firrtl.clock, in r: !firrtl.asyncreset)
   %c0_ui1 = firrtl.constant 0 : !firrtl.const.uint<1>
   %0 = firrtl.asAsyncReset %c0_ui1 : (!firrtl.const.uint<1>) -> !firrtl.const.asyncreset
   %1 = firrtl.constCast %0 : (!firrtl.const.asyncreset) -> !firrtl.asyncreset
   firrtl.matchingconnect %bar_r, %1 : !firrtl.asyncreset
   firrtl.matchingconnect %bar_c, %c : !firrtl.clock
 }
 firrtl.module private @Bar(in %c: !firrtl.clock, in %r: !firrtl.asyncreset) {
   %reg = firrtl.reg %c : !firrtl.clock, !firrtl.uint<8>
 }
}

// -----// IR Dump After InferResets (firrtl-infer-resets) //----- //
firrtl.circuit "Foo" {
 firrtl.module @Foo(in %p: !firrtl.uint<1>, in %r: !firrtl.asyncreset [{class = "sifive.enterprise.firrtl.FullAsyncResetAnnotation"}], in %c: !firrtl.clock) attributes {convention = #firrtl<convention scalarized>} {
   %bar_c, %bar_r = firrtl.instance bar @Bar(in c: !firrtl.clock, in r: !firrtl.asyncreset)
   firrtl.matchingconnect %bar_r, %r : !firrtl.asyncreset
   %c0_ui1 = firrtl.constant 0 : !firrtl.const.uint<1>
   %0 = firrtl.asAsyncReset %c0_ui1 : (!firrtl.const.uint<1>) -> !firrtl.const.asyncreset
   %1 = firrtl.constCast %0 : (!firrtl.const.asyncreset) -> !firrtl.asyncreset
   firrtl.matchingconnect %bar_r, %1 : !firrtl.asyncreset
   firrtl.matchingconnect %bar_c, %c : !firrtl.clock
 }
 firrtl.module private @Bar(in %c: !firrtl.clock, in %r: !firrtl.asyncreset) {
   %c0_ui8 = firrtl.constant 0 : !firrtl.const.uint<8>
   %reg = firrtl.regreset %c, %r, %c0_ui8 : !firrtl.clock, !firrtl.asyncreset, !firrtl.const.uint<8>, !firrtl.uint<8>
 }
}

I would expect a new port to be added to Bar and it to be connected to Foo.r. This mistake happens because the logic in InferResetsPass::determineImpl tries to reuse existing reset ports in submodules, which it does by matching on the type and port name, which is not nearly enough. If we change the name of the port Bar.r to something other than r, then it will not reuse the port and will drive the correct reset signal to its target.