[ConvertToArcs] Allow ops with regions

Tweak the ConvertToArcs pass to allow operations with regions. The pass
would just give up on these ops until now. However, in most cases we can
handle these ops just fine, as long as we take care to properly gather
all values used in nested operations. Also mark all side-effecting ops
as arc breakers, to prevent them from being gobbled up into arc defines,
where the verifier would be unhappy about them.

Author: fabianschuiki

lib/Conversion/ConvertToArcs/ConvertToArcs.cpp

@@ -14,6 +14,7 @@
 #include "circt/Support/Namespace.h"
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/RegionUtils.h"
 #include "llvm/Support/Debug.h"
 
 #define DEBUG_TYPE "convert-to-arcs"
@@ -22,13 +23,17 @@ using namespace circt;
 using namespace arc;
 using namespace hw;
 using llvm::MapVector;
+using llvm::SmallSetVector;
 
 static bool isArcBreakingOp(Operation *op) {
+  if (isa<TapOp>(op))
+    return false;
   return op->hasTrait<OpTrait::ConstantLike>() ||
          isa<hw::InstanceOp, seq::CompRegOp, MemoryOp, MemoryReadPortOp,
              ClockedOpInterface, seq::InitialOp, seq::ClockGateOp,
              sim::DPICallOp>(op) ||
-         op->getNumResults() > 1;
+         op->getNumResults() > 1 || op->getNumRegions() > 0 ||
+         !mlir::isMemoryEffectFree(op);
 }
 
 static LogicalResult convertInitialValue(seq::CompRegOp reg,
@@ -102,8 +107,6 @@ LogicalResult Converter::runOnModule(HWModuleOp module) {
   for (Operation &op : *module.getBodyBlock()) {
     if (isa<seq::InitialOp>(&op))
       continue;
-    if (op.getNumRegions() > 0)
-      return op.emitOpError("has regions; not supported by ConvertToArcs");
     if (!isArcBreakingOp(&op) && !isa<hw::OutputOp>(&op))
       continue;
     arcBreakerIndices[&op] = arcBreakers.size();
@@ -132,32 +135,40 @@ LogicalResult Converter::runOnModule(HWModuleOp module) {
 }
 
 LogicalResult Converter::analyzeFanIn() {
-  SmallVector<std::tuple<Operation *, unsigned>> worklist;
+  SmallVector<std::tuple<Operation *, SmallVector<Value, 2>>> worklist;
+  SetVector<Value> seenOperands;
+  auto addToWorklist = [&](Operation *op) {
+    seenOperands.clear();
+    for (auto operand : op->getOperands())
+      seenOperands.insert(operand);
+    mlir::getUsedValuesDefinedAbove(op->getRegions(), seenOperands);
+    worklist.emplace_back(op, seenOperands.getArrayRef());
+  };
 
   // Seed the worklist and fanin masks with the arc breaking operations.
   faninMasks.clear();
   for (auto *op : arcBreakers) {
     unsigned index = arcBreakerIndices.lookup(op);
     auto mask = APInt::getOneBitSet(arcBreakers.size(), index);
     faninMasks[op] = mask;
-    worklist.push_back({op, 0});
+    addToWorklist(op);
   }
 
   // Establish a post-order among the operations.
   DenseSet<Operation *> seen;
   DenseSet<Operation *> finished;
   postOrder.clear();
   while (!worklist.empty()) {
-    auto &[op, operandIdx] = worklist.back();
-    if (operandIdx == op->getNumOperands()) {
+    auto &[op, operands] = worklist.back();
+    if (operands.empty()) {
       if (!isArcBreakingOp(op) && !isa<hw::OutputOp>(op))
         postOrder.push_back(op);
       finished.insert(op);
       seen.erase(op);
       worklist.pop_back();
       continue;
     }
-    auto operand = op->getOperand(operandIdx++); // advance to next operand
+    auto operand = operands.pop_back_val(); // advance to next operand
     auto *definingOp = operand.getDefiningOp();
     if (!definingOp || isArcBreakingOp(definingOp) ||
         finished.contains(definingOp))
@@ -166,7 +177,7 @@ LogicalResult Converter::analyzeFanIn() {
       definingOp->emitError("combinational loop detected");
       return failure();
     }
-    worklist.push_back({definingOp, 0});
+    addToWorklist(definingOp);
   }
   LLVM_DEBUG(llvm::dbgs() << "- Sorted " << postOrder.size() << " ops\n");
 
@@ -177,6 +188,8 @@ LogicalResult Converter::analyzeFanIn() {
   for (auto *op : llvm::reverse(postOrder)) {
     auto mask = APInt::getZero(arcBreakers.size());
     for (auto *user : op->getUsers()) {
+      while (user->getParentOp() != op->getParentOp())
+        user = user->getParentOp();
       auto it = faninMasks.find(user);
       if (it != faninMasks.end())
         mask |= it->second;

test/Conversion/ConvertToArcs/convert-to-arcs-errors.mlir

@@ -291,3 +291,26 @@ hw.module @ObserveWires(in %in1: i32, in %in2: i32, out out: i32) {
 //   CHECK-DAG:   [[RES:%.+]]:2 = arc.call @[[ARC_NAME]]({{.*}}) :
 //   CHECK-DAG:   arc.tap [[RES]]#0 {name = "z"} : i32
 //       CHECK:   hw.output %0#1 : i32
+
+// CHECK: arc.define [[ARC_ADD:@OpsWithRegions.+]](
+// CHECK-NEXT: comb.add
+
+// CHECK: arc.define [[ARC_SUB:@OpsWithRegions.+]](
+// CHECK-NEXT: comb.sub
+
+// CHECK: hw.module @OpsWithRegions
+hw.module @OpsWithRegions(in %a: i42, in %b: i42, in %c: i42, in %d: i42, out z: i42) {
+  // CHECK-DAG: [[ADD:%.+]] = arc.call [[ARC_ADD]](%a, %b)
+  %0 = comb.add %a, %b : i42
+  // CHECK-DAG: [[COMB:%.+]] = llhd.combinational -> i42 {
+  // CHECK-DAG:   [[MUL:%.+]] = comb.mul [[ADD]], %c
+  // CHECK-DAG:   llhd.yield [[MUL]]
+  %1 = llhd.combinational -> i42 {
+    %3 = comb.mul %0, %c : i42
+    llhd.yield %3 : i42
+  }
+  // CHECK-DAG: [[SUB:%.+]] = arc.call [[ARC_SUB]]([[COMB]], %d)
+  %2 = comb.sub %1, %d : i42
+  // CHECK: hw.output [[SUB]]
+  hw.output %2 : i42
+}

test/Conversion/ConvertToArcs/convert-to-arcs.mlir

@@ -14,6 +14,7 @@
 #include "circt/Conversion/ArcToLLVM.h"
 #include "circt/Conversion/CombToArith.h"
 #include "circt/Conversion/ConvertToArcs.h"
+#include "circt/Conversion/Passes.h"
 #include "circt/Conversion/SeqToSV.h"
 #include "circt/Dialect/Arc/ArcDialect.h"
 #include "circt/Dialect/Arc/ArcInterfaces.h"
@@ -639,6 +640,7 @@ int main(int argc, char **argv) {
 
     // Dialect passes:
     arc::registerPasses();
+    registerConvertToArcs();
   }
 
   // Register any pass manager command line options.