[FIRRTL] Change firrtl-lower-xmr to a walk

Change the way that the `LowerXMR` pass visits operations in modules from
just visiting operations in the body to a walk.  This is done to fix a bug
exposed when compiling inline layers that define probes.  Previously,
operations inside the `sv.ifdef` that this lowering creates are not
visited resulting in a `LowerXMR` pass failure.  This change future-proofs
this pass so that any new operations added to FIRRTL which may include
regions and blocks will do something sane.  This also makes the pass more
relocatable within the FIRRTL pass pipeline.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>

Author: seldridge

lib/Dialect/FIRRTL/Transforms/LowerXMR.cpp

@@ -95,8 +95,8 @@ class LowerXMRPass : public circt::firrtl::impl::LowerXMRBase<LowerXMRPass> {
     SmallVector<Operation *> forceAndReleaseOps;
     // The dataflow function, that propagates the reachable RefSendOp across
     // RefType Ops.
-    auto transferFunc = [&](Operation &op) -> LogicalResult {
-      return TypeSwitch<Operation *, LogicalResult>(&op)
+    auto transferFunc = [&](Operation *op) -> LogicalResult {
+      return TypeSwitch<Operation *, LogicalResult>(op)
           .Case<RefSendOp>([&](RefSendOp send) {
             // Get a reference to the actual signal to which the XMR will be
             // generated.
@@ -266,9 +266,14 @@ class LowerXMRPass : public circt::firrtl::impl::LowerXMRBase<LowerXMRPass> {
       if (module.isPublic())
         publicModules.push_back(module);
 
-      for (Operation &op : module.getBodyBlock()->getOperations())
+      auto result = module.walk([&](Operation *op) {
         if (transferFunc(op).failed())
-          return signalPassFailure();
+          return WalkResult::interrupt();
+        return WalkResult::advance();
+      });
+
+      if (result.wasInterrupted())
+        return signalPassFailure();
 
       // Clear any enabled layers.
       module.setLayersAttr(ArrayAttr::get(module.getContext(), {}));

test/Dialect/FIRRTL/lowerXMR.mlir

@@ -780,3 +780,22 @@ firrtl.circuit "WireProbe" {
     firrtl.ref.define %p, %w : !firrtl.probe<uint<5>>
   }
 }
+
+// -----
+// Test that operations with regions and blocks are visited.  This will error if
+// blocks are not visited.
+
+// CHECK-LABEL: "Foo"
+firrtl.circuit "Foo" {
+  // CHECK: hierpath {{.*}} [@Foo::@[[SYM:[^ ]+]]]
+  sv.macro.decl @A
+  firrtl.module @Foo(out %a: !firrtl.probe<uint<1>>) {
+    //CHECK: sv.ifdef
+    sv.ifdef @A {
+      %b = firrtl.wire : !firrtl.uint<1>
+      // CHECK: firrtl.node sym @[[SYM]] {{.*}} %b
+      %0 = firrtl.ref.send %b : !firrtl.uint<1>
+      firrtl.ref.define %a, %0 : !firrtl.probe<uint<1>>
+    }
+  }
+}