[FIRRTL] FullAsyncResetTransform: does not properly find dominating block to insert reset signal

[youngar]

Given:

FIRRTL version 3.0.0
circuit Foo: %[[
  {"class":"sifive.enterprise.firrtl.FullAsyncResetAnnotation", "target":"~Foo|Foo>reset"}
]]
  module Foo:
    input p : UInt<1>
    input r : AsyncReset
    when p:
      wire reset : AsyncReset
      inst bar of Bar
    inst bar of Bar

  module Bar:

which if compiled with firtool test.fir gives the following error:

 ./fart2.fir:11:5: error: operand #1 does not dominate this use
    inst bar of Bar
    ^
./fart2.fir:11:5: note: see current operation: "firrtl.matchingconnect"(%0, %1) : (!firrtl.asyncreset, !firrtl.asyncreset) -> ()
./fart2.fir:9:7: note: operand defined here (op in a child region)
      wire reset : AsyncReset

The issue is that it attempts to connect the wire reset to an input port the second instance of bar. There is logic for handling things of this sort, but it seems to only care about the first operation we need to wire the signal to, instead of looking at all operations. https://github.com/llvm/circt/blob/281dc6cfa3638a4f77aba9a9f07bf2193bdc3959/lib/Dialect/FIRRTL/Transforms/InferResets.cpp#L1730-L1760

[dtzSiFive]

Thanks for filing this! Yes, this is a problem in most places that try to wire things around. Not that that's good.

As mentioned in multiple other places, FIRRTL is fundamentally incapable of plumbing between things that are declared under when's.

You /could/ do this with probe+ref.define+ref.resolve to fake an unconditional connect, but let's not? Probably we just need a "connect" that does what we want for normal signals.

(even if the wire was placed where it "dominated" (this doesn't really make sense for FIRRTL since it's just not that sort of thing) it'd still be conditionally connected to).