[Arc][ArcToLLVM] Fix seq.clock_gate lowering

This op did not properly lower because it created comb ops for which no lowering pattern was registered. Instead, lower to LLVM directly, but also register the CombToArith patterns to support comb as intermediate operations. This is also consistent with registering HWToLLVM and CombToLLVM in this pass directly. Also, add a missing test for the clock gate lowering.
llvm · Dec 31, 2024 · afa566a · afa566a
1 parent bae447c
commit afa566a
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 5 deletions.
diff --git a/lib/Conversion/ArcToLLVM/CMakeLists.txt b/lib/Conversion/ArcToLLVM/CMakeLists.txt
@@ -11,6 +11,7 @@ add_circt_conversion_library(CIRCTArcToLLVM
   CIRCTArc
   CIRCTComb
   CIRCTSeq
+  CIRCTCombToArith
   CIRCTCombToLLVM
   CIRCTHWToLLVM
   MLIRArithToLLVM

diff --git a/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp b/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "circt/Conversion/ArcToLLVM.h"
+#include "circt/Conversion/CombToArith.h"
 #include "circt/Conversion/CombToLLVM.h"
 #include "circt/Conversion/HWToLLVM.h"
 #include "circt/Dialect/Arc/ArcOps.h"
@@ -259,8 +260,8 @@ struct ClockGateOpLowering : public OpConversionPattern<seq::ClockGateOp> {
   LogicalResult
   matchAndRewrite(seq::ClockGateOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const final {
-    rewriter.replaceOpWithNewOp<comb::AndOp>(op, adaptor.getInput(),
-                                             adaptor.getEnable(), true);
+    rewriter.replaceOpWithNewOp<LLVM::AndOp>(op, adaptor.getInput(),
+                                             adaptor.getEnable());
     return success();
   }
 };
@@ -648,6 +649,7 @@ void LowerArcToLLVMPass::runOnOperation() {
   populateHWToLLVMConversionPatterns(converter, patterns, globals,
                                      constAggregateGlobalsMap);
   populateHWToLLVMTypeConversions(converter);
+  populateCombToArithConversionPatterns(converter, patterns);
   populateCombToLLVMConversionPatterns(converter, patterns);
 
   // Arc patterns.

diff --git a/test/Conversion/ArcToLLVM/lower-arc-to-llvm.mlir b/test/Conversion/ArcToLLVM/lower-arc-to-llvm.mlir
@@ -180,14 +180,16 @@ func.func @seqClocks(%clk1: !seq.clock, %clk2: !seq.clock) -> !seq.clock {
   %2 = arith.xori %0, %1 : i1
   %3 = seq.to_clock %2
   %4 = seq.clock_inv %3
-  return %4 : !seq.clock
+  %5 = seq.clock_gate %4, %0
+  return %5 : !seq.clock
 }
 // CHECK-LABEL: llvm.func @seqClocks
 //  CHECK-SAME: ([[CLK1:%.+]]: i1, [[CLK2:%.+]]: i1)
 //       CHECK: [[RES:%.+]] = llvm.xor [[CLK1]], [[CLK2]]
 //       CHECK: [[TRUE:%.+]] = llvm.mlir.constant(true) : i1
 //       CHECK: [[RES1:%.+]] = llvm.xor [[RES]], [[TRUE]] : i1
-//       CHECK: llvm.return [[RES1]] : i1
+//       CHECK: [[RES2:%.+]] = llvm.and [[RES1]], [[CLK1]] : i1
+//       CHECK: llvm.return [[RES2]] : i1
 
 // CHECK-LABEL: llvm.func @ReadAggregates(
 // CHECK-SAME: %arg0: !llvm.ptr

diff --git a/tools/arcilator/arcilator.cpp b/tools/arcilator/arcilator.cpp
@@ -355,7 +355,6 @@ static void populateArcToLLVMPipeline(PassManager &pm) {
   // Lower the arcs and update functions to LLVM.
   if (untilReached(UntilLLVMLowering))
     return;
-  pm.addPass(createConvertCombToArithPass());
   pm.addPass(createLowerArcToLLVMPass());
   pm.addPass(createCSEPass());
   pm.addPass(arc::createArcCanonicalizerPass());