From 5838a9ed2f9443bfb85b253bf0c9fef06de35f16 Mon Sep 17 00:00:00 2001 From: Fabian Schuiki Date: Thu, 7 Dec 2023 17:14:59 -0800 Subject: [PATCH] [Arc] Use seq.clock_gate op (#6501) The arc dialect currently provides its own `arc.clock_gate` operation. Since the seq dialect has a proper `seq.clock_gate` now, switch over to that and remove custom arc op. Fixes #6500. --- include/circt/Dialect/Arc/ArcOps.td | 9 ------- lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp | 4 +-- .../ConvertToArcs/ConvertToArcs.cpp | 4 +-- lib/Dialect/Arc/Transforms/LowerState.cpp | 26 ++++++++++++++++--- lib/Dialect/Arc/Transforms/StripSV.cpp | 11 ++++---- test/Dialect/Arc/lower-state.mlir | 11 +++++--- 6 files changed, 39 insertions(+), 26 deletions(-) diff --git a/include/circt/Dialect/Arc/ArcOps.td b/include/circt/Dialect/Arc/ArcOps.td index ddfe77746586..85b86070109c 100644 --- a/include/circt/Dialect/Arc/ArcOps.td +++ b/include/circt/Dialect/Arc/ArcOps.td @@ -260,15 +260,6 @@ def CallOp : ArcOp<"call", [ }]; } -def ClockGateOp : ArcOp<"clock_gate", [Pure]> { - let summary = "Clock gate"; - let arguments = (ins ClockType:$input, I1:$enable); - let results = (outs ClockType:$output); - let assemblyFormat = [{ - $input `,` $enable attr-dict - }]; -} - def MemoryOp : ArcOp<"memory", [MemoryEffects<[MemAlloc]>]> { let summary = "Memory"; let results = (outs MemoryType:$memory); diff --git a/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp b/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp index 253bb5e5b822..2531f8abc273 100644 --- a/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp +++ b/lib/Conversion/ArcToLLVM/LowerArcToLLVM.cpp @@ -240,10 +240,10 @@ struct MemoryWriteOpLowering : public OpConversionPattern { }; /// A dummy lowering for clock gates to an AND gate. -struct ClockGateOpLowering : public OpConversionPattern { +struct ClockGateOpLowering : public OpConversionPattern { using OpConversionPattern::OpConversionPattern; LogicalResult - matchAndRewrite(arc::ClockGateOp op, OpAdaptor adaptor, + matchAndRewrite(seq::ClockGateOp op, OpAdaptor adaptor, ConversionPatternRewriter &rewriter) const final { rewriter.replaceOpWithNewOp(op, adaptor.getInput(), adaptor.getEnable(), true); diff --git a/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp b/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp index 8cedd5fdf90d..a502294c9f8d 100644 --- a/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp +++ b/lib/Conversion/ConvertToArcs/ConvertToArcs.cpp @@ -23,8 +23,8 @@ using llvm::MapVector; static bool isArcBreakingOp(Operation *op) { return op->hasTrait() || - isa(op) || + isa(op) || op->getNumResults() > 1; } diff --git a/lib/Dialect/Arc/Transforms/LowerState.cpp b/lib/Dialect/Arc/Transforms/LowerState.cpp index 76f3675c5545..298160c6086d 100644 --- a/lib/Dialect/Arc/Transforms/LowerState.cpp +++ b/lib/Dialect/Arc/Transforms/LowerState.cpp @@ -72,6 +72,8 @@ struct ClockLowering { IRMapping materializedValues; /// A cache of AND gates created for aggregating enable conditions. DenseMap, Value> andCache; + /// A cache of OR gates created for aggregating enable conditions. + DenseMap, Value> orCache; ClockLowering(Value clock, Operation *treeOp, Statistics &stats) : clock(clock), treeOp(treeOp), stats(stats), builder(treeOp) { @@ -81,6 +83,7 @@ struct ClockLowering { Value materializeValue(Value value); Value getOrCreateAnd(Value lhs, Value rhs, Location loc); + Value getOrCreateOr(Value lhs, Value rhs, Location loc); }; struct GatedClockLowering { @@ -236,13 +239,27 @@ Value ClockLowering::getOrCreateAnd(Value lhs, Value rhs, Location loc) { return slot; } +/// Create an OR gate if none with the given operands already exists. Note that +/// the operands may be null, in which case the function will return the +/// non-null operand, or null if both operands are null. +Value ClockLowering::getOrCreateOr(Value lhs, Value rhs, Location loc) { + if (!lhs) + return rhs; + if (!rhs) + return lhs; + auto &slot = orCache[std::make_pair(lhs, rhs)]; + if (!slot) + slot = builder.create(loc, lhs, rhs); + return slot; +} + //===----------------------------------------------------------------------===// // Module Lowering //===----------------------------------------------------------------------===// GatedClockLowering ModuleLowering::getOrCreateClockLowering(Value clock) { // Look through clock gates. - if (auto ckgOp = clock.getDefiningOp()) { + if (auto ckgOp = clock.getDefiningOp()) { // Reuse the existing lowering for this clock gate if possible. if (auto it = gatedClockLowerings.find(clock); it != gatedClockLowerings.end()) @@ -254,8 +271,11 @@ GatedClockLowering ModuleLowering::getOrCreateClockLowering(Value clock) { // we have to do is to add this clock gate's condition to that list. auto info = getOrCreateClockLowering(ckgOp.getInput()); auto ckgEnable = info.clock.materializeValue(ckgOp.getEnable()); - info.enable = - info.clock.getOrCreateAnd(info.enable, ckgEnable, ckgOp.getLoc()); + auto ckgTestEnable = info.clock.materializeValue(ckgOp.getTestEnable()); + info.enable = info.clock.getOrCreateAnd( + info.enable, + info.clock.getOrCreateOr(ckgEnable, ckgTestEnable, ckgOp.getLoc()), + ckgOp.getLoc()); gatedClockLowerings.insert({clock, info}); return info; } diff --git a/lib/Dialect/Arc/Transforms/StripSV.cpp b/lib/Dialect/Arc/Transforms/StripSV.cpp index e1f30c5a1182..463489126fb9 100644 --- a/lib/Dialect/Arc/Transforms/StripSV.cpp +++ b/lib/Dialect/Arc/Transforms/StripSV.cpp @@ -158,16 +158,15 @@ void StripSVPass::runOnOperation() { continue; } - // Replace clock gate instances with the dedicated arc op and stub - // out other external modules. + // Replace clock gate instances with the dedicated `seq.clock_gate` op and + // stub out other external modules. if (auto instOp = dyn_cast(&op)) { auto modName = instOp.getModuleNameAttr().getAttr(); ImplicitLocOpBuilder builder(instOp.getLoc(), instOp); if (clockGateModuleNames.contains(modName)) { - auto enable = builder.createOrFold( - instOp.getOperand(1), instOp.getOperand(2), true); - auto gated = - builder.create(instOp.getOperand(0), enable); + auto gated = builder.create( + instOp.getOperand(0), instOp.getOperand(1), instOp.getOperand(2), + hw::InnerSymAttr{}); instOp.replaceAllUsesWith(gated); opsToDelete.push_back(instOp); } diff --git a/test/Dialect/Arc/lower-state.mlir b/test/Dialect/Arc/lower-state.mlir index 1275e1e1ebdb..927326887196 100644 --- a/test/Dialect/Arc/lower-state.mlir +++ b/test/Dialect/Arc/lower-state.mlir @@ -28,8 +28,8 @@ hw.module @InputsAndOutputs(in %a: i42, in %b: i17, out c: i42, out d: i17) { } // CHECK-LABEL: arc.model "State" { -hw.module @State(in %clk: !seq.clock, in %en: i1) { - %gclk = arc.clock_gate %clk, %en +hw.module @State(in %clk: !seq.clock, in %en: i1, in %en2: i1) { + %gclk = seq.clock_gate %clk, %en, %en2 %3 = arc.state @DummyArc(%6) clock %clk lat 1 : (i42) -> i42 %4 = arc.state @DummyArc(%5) clock %gclk lat 1 : (i42) -> i42 %5 = comb.add %3, %3 : i42 @@ -37,6 +37,7 @@ hw.module @State(in %clk: !seq.clock, in %en: i1) { // CHECK-NEXT: (%arg0: !arc.storage): // CHECK-NEXT: [[INCLK:%.+]] = arc.root_input "clk", %arg0 : (!arc.storage) -> !arc.state // CHECK-NEXT: [[INEN:%.+]] = arc.root_input "en", %arg0 : (!arc.storage) -> !arc.state + // CHECK-NEXT: [[INEN2:%.+]] = arc.root_input "en2", %arg0 : (!arc.storage) -> !arc.state // CHECK-NEXT: [[CLK_OLD:%.+]] = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state // CHECK-NEXT: [[S0:%.+]] = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state // CHECK-NEXT: [[S1:%.+]] = arc.alloc_state %arg0 : (!arc.storage) -> !arc.state @@ -52,11 +53,13 @@ hw.module @State(in %clk: !seq.clock, in %en: i1) { // CHECK-NEXT: [[TMP1:%.+]] = comb.add [[TMP0]], [[TMP0]] // CHECK-NEXT: [[TMP2:%.+]] = arc.state @DummyArc([[TMP1]]) lat 0 : (i42) -> i42 // CHECK-NEXT: arc.state_write [[S0]] = [[TMP2]] : + // CHECK-NEXT: [[EN:%.+]] = arc.state_read [[INEN]] : + // CHECK-NEXT: [[EN2:%.+]] = arc.state_read [[INEN2]] : + // CHECK-NEXT: [[TMP3:%.+]] = comb.or [[EN]], [[EN2]] : i1 // CHECK-NEXT: [[TMP0:%.+]] = arc.state_read [[S0]] : // CHECK-NEXT: [[TMP1:%.+]] = comb.add [[TMP0]], [[TMP0]] // CHECK-NEXT: [[TMP2:%.+]] = arc.state @DummyArc([[TMP1]]) lat 0 : (i42) -> i42 - // CHECK-NEXT: [[EN:%.+]] = arc.state_read [[INEN]] : - // CHECK-NEXT: arc.state_write [[S1]] = [[TMP2]] if [[EN]] : + // CHECK-NEXT: arc.state_write [[S1]] = [[TMP2]] if [[TMP3]] : // CHECK-NEXT: } }