From 6a4849d043032ace9c891e04eae50c0240a5c1e5 Mon Sep 17 00:00:00 2001 From: Morten Borup Petersen Date: Mon, 13 Nov 2023 15:57:16 +0100 Subject: [PATCH] [Seq] Add shiftreg op (#6038) Adds a shift register operation to the seq dialect. The operation has an interface similar to a `seq.compreg.ce` operation, with an additional `size` to specify the number of stages. Included is a default lowering to `seq.compreg.ce` operations. The main intention of introducing this operation is to be able to emit shift registers using target-specific resources. I don't see a reason to add a non-clock-enabled shift reg operation in addition to this. If users need a non-clock-enabled shift register, it should suffice to emit a shift register with constant 1 clock-enable, whereafter (if lowered to e.g. `seq.compreg.ce`), the lowered primitive should have a canonicalization that considers the constant clock enable signal. --- include/circt/Dialect/Seq/SeqOps.td | 43 +++++++++++ include/circt/Dialect/Seq/SeqPasses.h | 1 + include/circt/Dialect/Seq/SeqPasses.td | 15 ++++ lib/Conversion/PipelineToHW/PipelineToHW.cpp | 3 +- lib/Dialect/Seq/SeqOps.cpp | 39 ++++++++-- lib/Dialect/Seq/Transforms/CMakeLists.txt | 1 + .../Seq/Transforms/LowerSeqShiftReg.cpp | 74 +++++++++++++++++++ test/Dialect/Seq/shiftreg.mlir | 20 +++++ 8 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 lib/Dialect/Seq/Transforms/LowerSeqShiftReg.cpp create mode 100644 test/Dialect/Seq/shiftreg.mlir diff --git a/include/circt/Dialect/Seq/SeqOps.td b/include/circt/Dialect/Seq/SeqOps.td index 03ec9ef4b467..c7214071fe1d 100644 --- a/include/circt/Dialect/Seq/SeqOps.td +++ b/include/circt/Dialect/Seq/SeqOps.td @@ -123,6 +123,49 @@ def CompRegClockEnabledOp : SeqOp<"compreg.ce", ]; } +def ShiftRegOp : SeqOp<"shiftreg", [ + Pure, + Clocked, + Resettable, + AllTypesMatch<["input", "data"]>, + DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, + AttrSizedOperandSegments +]> { + let summary = "Shift register"; + let description = [{ + The `seq.shiftreg` op represents a shift register. It takes the input + value and shifts it every cycle when `clockEnable` is asserted. + The `reset` and `resetValue` operands are optional and if present, every + entry in the shift register will be initialized to `resetValue` upon + assertion of the reset signal. Exact reset behavior (sync/async) is + implementation defined. + }]; + + let arguments = (ins + ConfinedAttr]>:$numElements, + AnyType:$input, + ClockType:$clk, + I1:$clockEnable, + OptionalAttr:$name, + Optional:$reset, + Optional:$resetValue, + Optional:$powerOnValue, + OptionalAttr:$inner_sym + ); + let results = (outs AnyType:$data); + let hasVerifier = 1; + + let assemblyFormat = [{ + `[` $numElements `]` + (`sym` $inner_sym^)? custom($name) $input `,` $clk `,` $clockEnable + (`reset` $reset^ `,` $resetValue)? + (`powerOn` $powerOnValue^)? attr-dict `:` type($data) + custom(ref(type($data)), ref($resetValue), type($resetValue)) + custom(ref(type($data)), ref($powerOnValue), type($powerOnValue)) + }]; +} + //===----------------------------------------------------------------------===// // FIRRTL-flavored register //===----------------------------------------------------------------------===// diff --git a/include/circt/Dialect/Seq/SeqPasses.h b/include/circt/Dialect/Seq/SeqPasses.h index 0893411ede8c..1da12e2058a9 100644 --- a/include/circt/Dialect/Seq/SeqPasses.h +++ b/include/circt/Dialect/Seq/SeqPasses.h @@ -29,6 +29,7 @@ createExternalizeClockGatePass(const ExternalizeClockGateOptions &options = {}); std::unique_ptr createLowerSeqFIFOPass(); std::unique_ptr createHWMemSimImplPass(const HWMemSimImplOptions &options = {}); +std::unique_ptr createLowerSeqShiftRegPass(); /// Generate the code for registering passes. #define GEN_PASS_REGISTRATION diff --git a/include/circt/Dialect/Seq/SeqPasses.td b/include/circt/Dialect/Seq/SeqPasses.td index 162dc65a04f2..9d63c79b0558 100644 --- a/include/circt/Dialect/Seq/SeqPasses.td +++ b/include/circt/Dialect/Seq/SeqPasses.td @@ -80,4 +80,19 @@ def HWMemSimImpl : Pass<"hw-memory-sim", "ModuleOp"> { ]; } +def LowerSeqShiftReg : Pass<"lower-seq-shiftreg", "hw::HWModuleOp"> { + let summary = "Lower seq.shiftreg ops"; + let description = [{ + Default pass for lowering shift register operations. This will lower + into a chain of `seq.compreg.ce` operations. + Note that this is _not_ guaranteed to be a performant implementation, + but instead a default, fallback lowering path which is guaranteed to + provide a semantically valid path to verilog emissions. + Users are expected to provide a custom lowering pass to maps `seq.shiftreg` + operations to target-specific primitives. + }]; + let constructor = "circt::seq::createLowerSeqShiftRegPass()"; + let dependentDialects = ["circt::hw::HWDialect"]; +} + #endif // CIRCT_DIALECT_SEQ_SEQPASSES diff --git a/lib/Conversion/PipelineToHW/PipelineToHW.cpp b/lib/Conversion/PipelineToHW/PipelineToHW.cpp index dcc4ed48130b..700ee39c2459 100644 --- a/lib/Conversion/PipelineToHW/PipelineToHW.cpp +++ b/lib/Conversion/PipelineToHW/PipelineToHW.cpp @@ -184,8 +184,7 @@ class PipelineLowering { // be clocked. if (isStallablePipeline) { dataReg = builder.create( - stageOp->getLoc(), regIn, args.clock, stageValid, - regName.strref()); + stageOp->getLoc(), regIn, args.clock, stageValid, regName); } else { dataReg = builder.create(stageOp->getLoc(), regIn, args.clock, regName); diff --git a/lib/Dialect/Seq/SeqOps.cpp b/lib/Dialect/Seq/SeqOps.cpp index 62c627ae53ba..b77a3da6667b 100644 --- a/lib/Dialect/Seq/SeqOps.cpp +++ b/lib/Dialect/Seq/SeqOps.cpp @@ -299,8 +299,6 @@ void CompRegOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) { setNameFn(getResult(), *name); } -std::optional CompRegOp::getTargetResultIndex() { return 0; } - LogicalResult CompRegOp::verify() { if ((getReset() == nullptr) ^ (getResetValue() == nullptr)) return emitOpError( @@ -308,6 +306,20 @@ LogicalResult CompRegOp::verify() { return success(); } +std::optional CompRegOp::getTargetResultIndex() { return 0; } + +template +LogicalResult verifyResets(TOp op) { + if ((op.getReset() == nullptr) ^ (op.getResetValue() == nullptr)) + return op->emitOpError( + "either reset and resetValue or neither must be specified"); + bool hasReset = op.getReset() != nullptr; + if (hasReset && op.getResetValue().getType() != op.getInput().getType()) + return op->emitOpError("reset value must be the same type as the input"); + + return success(); +} + /// Suggest a name for each result value based on the saved result names /// attribute. void CompRegClockEnabledOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) { @@ -321,9 +333,26 @@ std::optional CompRegClockEnabledOp::getTargetResultIndex() { } LogicalResult CompRegClockEnabledOp::verify() { - if ((getReset() == nullptr) ^ (getResetValue() == nullptr)) - return emitOpError( - "either reset and resetValue or neither must be specified"); + if (failed(verifyResets(*this))) + return failure(); + return success(); +} + +//===----------------------------------------------------------------------===// +// ShiftRegOp +//===----------------------------------------------------------------------===// + +void ShiftRegOp::getAsmResultNames(OpAsmSetValueNameFn setNameFn) { + // If the wire has an optional 'name' attribute, use it. + if (auto name = getName()) + setNameFn(getResult(), *name); +} + +std::optional ShiftRegOp::getTargetResultIndex() { return 0; } + +LogicalResult ShiftRegOp::verify() { + if (failed(verifyResets(*this))) + return failure(); return success(); } diff --git a/lib/Dialect/Seq/Transforms/CMakeLists.txt b/lib/Dialect/Seq/Transforms/CMakeLists.txt index 49783678f6b0..07e59d456101 100644 --- a/lib/Dialect/Seq/Transforms/CMakeLists.txt +++ b/lib/Dialect/Seq/Transforms/CMakeLists.txt @@ -3,6 +3,7 @@ add_circt_dialect_library(CIRCTSeqTransforms HWMemSimImpl.cpp LowerSeqHLMem.cpp LowerSeqFIFO.cpp + LowerSeqShiftReg.cpp DEPENDS CIRCTSeqTransformsIncGen diff --git a/lib/Dialect/Seq/Transforms/LowerSeqShiftReg.cpp b/lib/Dialect/Seq/Transforms/LowerSeqShiftReg.cpp new file mode 100644 index 000000000000..0dd2812cecd3 --- /dev/null +++ b/lib/Dialect/Seq/Transforms/LowerSeqShiftReg.cpp @@ -0,0 +1,74 @@ +//===- LowerSeqShiftReg.cpp - seq.shiftreg lowering -----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "PassDetails.h" +#include "circt/Dialect/Comb/CombOps.h" +#include "circt/Dialect/SV/SVOps.h" +#include "circt/Dialect/Seq/SeqOps.h" +#include "circt/Dialect/Seq/SeqPasses.h" +#include "circt/Support/BackedgeBuilder.h" +#include "mlir/Transforms/DialectConversion.h" +#include "llvm/ADT/TypeSwitch.h" + +using namespace circt; +using namespace seq; + +namespace { + +struct ShiftRegLowering : public OpConversionPattern { +public: + using OpConversionPattern::OpConversionPattern; + + LogicalResult + matchAndRewrite(seq::ShiftRegOp op, OpAdaptor adaptor, + ConversionPatternRewriter &rewriter) const final { + Value in = adaptor.getInput(); + auto baseName = op.getName(); + for (size_t i = 0; i < op.getNumElements(); ++i) { + StringAttr name; + if (baseName.has_value()) + name = rewriter.getStringAttr(baseName.value() + "_sh" + Twine(i + 1)); + in = rewriter.create( + op.getLoc(), in, adaptor.getClk(), adaptor.getClockEnable(), + adaptor.getReset(), adaptor.getResetValue(), name, + op.getPowerOnValue()); + } + + op.replaceAllUsesWith(in); + rewriter.eraseOp(op); + return success(); + } +}; + +#define GEN_PASS_DEF_LOWERSEQSHIFTREG +#include "circt/Dialect/Seq/SeqPasses.h.inc" + +struct LowerSeqShiftRegPass + : public impl::LowerSeqShiftRegBase { + void runOnOperation() override; +}; + +} // namespace + +void LowerSeqShiftRegPass::runOnOperation() { + MLIRContext &ctxt = getContext(); + ConversionTarget target(ctxt); + + target.addIllegalOp(); + target.addLegalDialect(); + RewritePatternSet patterns(&ctxt); + patterns.add(&ctxt); + + if (failed( + applyPartialConversion(getOperation(), target, std::move(patterns)))) + signalPassFailure(); +} + +std::unique_ptr circt::seq::createLowerSeqShiftRegPass() { + return std::make_unique(); +} diff --git a/test/Dialect/Seq/shiftreg.mlir b/test/Dialect/Seq/shiftreg.mlir new file mode 100644 index 000000000000..eda06e4a7ce1 --- /dev/null +++ b/test/Dialect/Seq/shiftreg.mlir @@ -0,0 +1,20 @@ +// RUN: circt-opt %s | circt-opt | FileCheck %s +// RUN: circt-opt --lower-seq-shiftreg %s | FileCheck %s --check-prefix=LO + +// CHECK: %r0 = seq.shiftreg[3] %i, %clk, %ce : i32 +// CHECK: %myShiftReg = seq.shiftreg[3] sym @myShiftReg %i, %clk, %ce reset %rst, %c0_i32 powerOn %c0_i32 : i32 + +// LO: %r0_sh1 = seq.compreg.ce sym @r0_sh1 %i, %clk, %ce : i32 +// LO: %r0_sh2 = seq.compreg.ce sym @r0_sh2 %r0_sh1, %clk, %ce : i32 +// LO: %r0_sh3 = seq.compreg.ce sym @r0_sh3 %r0_sh2, %clk, %ce : i32 +// LO: %myShiftReg_sh1 = seq.compreg.ce sym @myShiftReg_sh1 %i, %clk, %ce reset %rst, %c0_i32 powerOn %c0_i32 : i32 +// LO: %myShiftReg_sh2 = seq.compreg.ce sym @myShiftReg_sh2 %myShiftReg_sh1, %clk, %ce reset %rst, %c0_i32 powerOn %c0_i32 : i32 +// LO: %myShiftReg_sh3 = seq.compreg.ce sym @myShiftReg_sh3 %myShiftReg_sh2, %clk, %ce reset %rst, %c0_i32 powerOn %c0_i32 : i32 +// LO: hw.output %r0_sh3, %myShiftReg_sh3 : i32, i32 + +hw.module @top(in %clk: !seq.clock, in %rst: i1, in %ce: i1, in %i: i32, out out1 : i32, out out2 : i32) { + %rv = hw.constant 0 : i32 + %r0 = seq.shiftreg [3] %i, %clk, %ce : i32 + %myShiftReg = seq.shiftreg [3] sym @myShiftReg %i, %clk, %ce reset %rst, %rv powerOn %rv : i32 + hw.output %r0, %myShiftReg : i32, i32 +}