From 6538b1e8373efe3a0a2ced60108015677c44864c Mon Sep 17 00:00:00 2001
From: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Date: Tue, 3 Dec 2024 17:29:38 -0500
Subject: [PATCH] [FIRRTLToHW] Walk regions/blocks when converting

Change the `LowerToHW` FIRRTL to HW conversion pass to walk
`firrtl.module` bodies as opposed to just visiting operations in their
bodies.  This has the benefit of allowing intermixing of operations from
other dialects, e.g., `sv.ifdef`, which may be generated by passes right
before this conversion runs.

Practically, this is part of turning on inline layer support for FIRRTL
compiler pipelines.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
---
 lib/Conversion/FIRRTLToHW/LowerToHW.cpp     | 24 ++++++++++++---------
 test/Conversion/FIRRTLToHW/lower-to-hw.mlir | 11 ++++++++++
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp
index 2855be4511ef..3b7b3ed731ec 100644
--- a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp
+++ b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp
@@ -1846,26 +1846,30 @@ LogicalResult FIRRTLLowering::run() {
 
   // Iterate through each operation in the module body, attempting to lower
   // each of them.  We maintain 'builder' for each invocation.
-  for (auto &op : body.front().getOperations()) {
-    builder.setInsertionPoint(&op);
-    builder.setLoc(op.getLoc());
-    auto done = succeeded(dispatchVisitor(&op));
-    circuitState.processRemainingAnnotations(&op, AnnotationSet(&op));
+  auto result = theModule.walk([&](Operation *op){
+    builder.setInsertionPoint(op);
+    builder.setLoc(op->getLoc());
+    auto done = succeeded(dispatchVisitor(op));
+    circuitState.processRemainingAnnotations(op, AnnotationSet(op));
     if (done)
-      opsToRemove.push_back(&op);
+      opsToRemove.push_back(op);
     else {
-      switch (handleUnloweredOp(&op)) {
+      switch (handleUnloweredOp(op)) {
       case AlreadyLowered:
         break;         // Something like hw.output, which is already lowered.
       case NowLowered: // Something handleUnloweredOp removed.
-        opsToRemove.push_back(&op);
+        opsToRemove.push_back(op);
         break;
       case LoweringFailure:
         backedgeBuilder.abandon();
-        return failure();
+        return WalkResult::interrupt();
       }
     }
-  }
+    return WalkResult::advance();
+  });
+
+  if (result.wasInterrupted())
+    return failure();
 
   // Replace all backedges with uses of their regular values.  We process them
   // after the module body since the lowering table is too hard to keep up to
diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir
index 06bce0aa0156..442902dc54f5 100644
--- a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir
+++ b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir
@@ -1480,6 +1480,17 @@ firrtl.circuit "Simple"   attributes {annotations = [{class =
     // CHECK-NEXT: %6 = sv.read_inout %5 : !hw.inout<i32>
     // CHECK-NEXT: hw.output %2, %6 : i32, i32
   }
+
+  sv.macro.decl @IfDef_MacroDecl
+  // CHECK-LABEL: @IfDef
+  firrtl.module @IfDef() {
+    // CHECK: sv.ifdef
+    sv.ifdef @IfDef_MacroDecl {
+      // CHECK-NEXT: %a = hw.wire
+      %a = firrtl.wire : !firrtl.uint<1>
+    }
+  }
+
 }
 
 // -----