From 0ca7d1112c1b2d084f067999d6dd4c76ce10ba90 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Tue, 1 Jul 2025 00:53:47 +0800 Subject: [PATCH 01/11] support firreg --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 51 +++++++++++++++++++ .../circt-bmc/externalize-registers.mlir | 45 ++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 3765c5cbfccf..a3aa82e5b7cc 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -150,6 +150,57 @@ void ExternalizeRegistersPass::runOnOperation() { ++numRegs; return; } + if (auto regOp = dyn_cast(op)) { + if (!isa(regOp.getClk())) { + regOp.emitError("only clocks directly given as block arguments " + "are supported"); + return signalPassFailure(); + } + mlir::Attribute initState; + if (auto preset = regOp.getPreset()) { + // Get preset value as initState + initState = mlir::IntegerAttr::get( + mlir::IntegerType::get(&getContext(), preset->getBitWidth()), + *preset); + } else { + // If there's no preset value just add a unit attribute to maintain + // one-to-one correspondence with module ports + initState = mlir::UnitAttr::get(&getContext()); + } + addedInputs[module.getSymNameAttr()].push_back(regOp.getType()); + addedOutputs[module.getSymNameAttr()].push_back( + regOp.getNext().getType()); + OpBuilder builder(regOp); + auto regName = regOp.getName(); + StringAttr newInputName, newOutputName; + if (!regName.empty()) { + newInputName = builder.getStringAttr(regName + "_state"); + newOutputName = builder.getStringAttr(regName + "_input"); + } else { + newInputName = + builder.getStringAttr("reg_" + Twine(numRegs) + "_state"); + newOutputName = + builder.getStringAttr("reg_" + Twine(numRegs) + "_input"); + } + addedInputNames[module.getSymNameAttr()].push_back(newInputName); + addedOutputNames[module.getSymNameAttr()].push_back(newOutputName); + initialValues[module.getSymNameAttr()].push_back(initState); + + regOp.getResult().replaceAllUsesWith( + module.appendInput(newInputName, regOp.getType()).second); + if (auto reset = regOp.getReset()) { + auto resetValue = regOp.getResetValue(); + auto mux = + builder.create(regOp.getLoc(), regOp.getType(), + reset, resetValue, regOp.getNext()); + module.appendOutput(newOutputName, mux); + } else { + module.appendOutput(newOutputName, regOp.getNext()); + } + regOp->erase(); + ++numRegs; + return; + } if (auto instanceOp = dyn_cast(op)) { OpBuilder builder(instanceOp); auto newInputs = diff --git a/test/Tools/circt-bmc/externalize-registers.mlir b/test/Tools/circt-bmc/externalize-registers.mlir index 971420f9c562..1fdfc61f8762 100644 --- a/test/Tools/circt-bmc/externalize-registers.mlir +++ b/test/Tools/circt-bmc/externalize-registers.mlir @@ -98,3 +98,48 @@ hw.module @reg_with_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out %1 = seq.compreg %in, %clk reset %rst, %c0_i32 : i32 hw.output %1 : i32 } + +// ----- + +// CHECK: hw.module @one_firreg(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in [[OLD_REG:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [0 : i32], num_regs = 1 : i32} { +// CHECK: [[ADD:%.+]] = comb.add [[IN0]], [[IN1]] +// CHECK: hw.output [[OLD_REG]], [[ADD]] +// CHECK: } +hw.module @one_firreg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: i32) { + %0 = comb.add %in0, %in1 : i32 + %single_reg = seq.firreg %0 clock %clk preset 0 : i32 + hw.output %single_reg : i32 +} + +// CHECK: hw.module @combreg_and_firreg(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in [[OLD_REG0:%.+]] : i32, in [[OLD_REG1:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit, unit], num_regs = 2 : i32} { +// CHECK: [[ADD:%.+]] = comb.add [[IN0]], [[IN1]] +// CHECK: hw.output [[OLD_REG1]], [[ADD]], [[OLD_REG0]] +// CHECK: } +hw.module @combreg_and_firreg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: i32) { + %0 = comb.add %in0, %in1 : i32 + %1 = seq.compreg %0, %clk : i32 + %2 = seq.firreg %1 clock %clk : i32 + hw.output %2 : i32 +} + +// CHECK: hw.module @named_firregs(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in %firstreg_state : i32, in %secondreg_state : i32, out {{.+}} : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit, unit], num_regs = 2 : i32} { +// CHECK: [[ADD:%.+]] = comb.add [[IN0]], [[IN1]] +// CHECK: hw.output %secondreg_state, [[ADD]], %firstreg_state +// CHECK: } +hw.module @named_firregs(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: i32) { + %0 = comb.add %in0, %in1 : i32 + %firstreg = seq.firreg %0 clock %clk : i32 + %secondreg = seq.firreg %firstreg clock %clk : i32 + hw.output %secondreg : i32 +} + +// CHECK: hw.module @firreg_with_reset(in [[CLK:%.+]] : !seq.clock, in [[RST:%.+]] : i1, in [[IN:%.+]] : i32, in [[OLD_REG:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit], num_regs = 1 : i32} { +// CHECK: [[C0_I32:%.+]] = hw.constant 0 : i32 +// CHECK: [[MUX:%.+]] = comb.mux [[RST]], [[C0_I32]], [[IN]] : i32 +// CHECK: hw.output [[OLD_REG]], [[MUX]] +// CHECK: } +hw.module @firreg_with_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out: i32) { + %c0_i32 = hw.constant 0 : i32 + %1 = seq.firreg %in clock %clk reset sync %rst, %c0_i32 : i32 + hw.output %1 : i32 +} From 9d720aa4e33b99fc557290019b09aefbc75ece7b Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Sat, 12 Jul 2025 18:42:48 +0800 Subject: [PATCH 02/11] emitError about async reset, add test case --- integration_test/circt-bmc/seq.mlir | 15 +++++++++++++++ lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 16 ++++++++++++++-- .../circt-bmc/externalize-registers-errors.mlir | 8 ++++++++ test/Tools/circt-bmc/externalize-registers.mlir | 8 ++++---- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/integration_test/circt-bmc/seq.mlir b/integration_test/circt-bmc/seq.mlir index 8c28e9a98df8..45841c876a65 100644 --- a/integration_test/circt-bmc/seq.mlir +++ b/integration_test/circt-bmc/seq.mlir @@ -70,3 +70,18 @@ hw.module @Counter_with_reset(in %clk: !seq.clock, in %rst: i1, out count: i3) { verif.assert %neq : i1 hw.output %reg : i3 } + +// Check that reset of firreg can be triggered +// RUN: circt-bmc %s -b 4 --module Counter_with_firreg_sync_reset --shared-libs=%libz3 | FileCheck %s --check-prefix=FIRREGRESET +// FIRREGRESET: Assertion can be violated! +hw.module @Counter_with_firreg_sync_reset(in %clk: !seq.clock, in %rst: i1, out count: i3) { + %c0_i3 = hw.constant 0 : i3 + %c1_i3 = hw.constant 1 : i3 + %regPlusOne = comb.add %reg, %c1_i3 : i3 + %reg = seq.firreg %regPlusOne clock %clk reset sync %rst, %c0_i3 preset 1 : i3 + %neq = comb.icmp bin ne %reg, %c0_i3 : i3 + // Assertion will be violated if the reset is triggered + verif.assert %neq : i1 + hw.output %reg : i3 +} + diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index a3aa82e5b7cc..d78c454b8ef4 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -186,17 +186,29 @@ void ExternalizeRegistersPass::runOnOperation() { addedOutputNames[module.getSymNameAttr()].push_back(newOutputName); initialValues[module.getSymNameAttr()].push_back(initState); - regOp.getResult().replaceAllUsesWith( - module.appendInput(newInputName, regOp.getType()).second); + // Replace the register with newInput and newOutput + auto newInput = + module.appendInput(newInputName, regOp.getType()).second; if (auto reset = regOp.getReset()) { auto resetValue = regOp.getResetValue(); + if (regOp.getIsAsync()) { + // Async reset + regOp.emitError("seq.firreg with async reset not yet supported"); + return signalPassFailure(); + } else { + // Sync reset + regOp.getResult().replaceAllUsesWith(newInput); + } auto mux = builder.create(regOp.getLoc(), regOp.getType(), reset, resetValue, regOp.getNext()); module.appendOutput(newOutputName, mux); } else { + // No reset + regOp.getResult().replaceAllUsesWith(newInput); module.appendOutput(newOutputName, regOp.getNext()); } + regOp->erase(); ++numRegs; return; diff --git a/test/Tools/circt-bmc/externalize-registers-errors.mlir b/test/Tools/circt-bmc/externalize-registers-errors.mlir index 37660d5f9f80..d57891a0e8c4 100644 --- a/test/Tools/circt-bmc/externalize-registers-errors.mlir +++ b/test/Tools/circt-bmc/externalize-registers-errors.mlir @@ -54,3 +54,11 @@ hw.module @reg_with_instance_initial(in %clk: !seq.clock, in %in: i32, out out: %1 = seq.compreg %in, %clk initial %init : i32 hw.output %1 : i32 } + +// ----- +hw.module @firreg_with_async_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out: i32) { + %c0_i32 = hw.constant 0 : i32 + // expected-error @below {{seq.firreg with async reset not yet supported}} + %1 = seq.firreg %in clock %clk reset async %rst, %c0_i32 : i32 + hw.output %1 : i32 +} \ No newline at end of file diff --git a/test/Tools/circt-bmc/externalize-registers.mlir b/test/Tools/circt-bmc/externalize-registers.mlir index 1fdfc61f8762..88b6e8b5cc7b 100644 --- a/test/Tools/circt-bmc/externalize-registers.mlir +++ b/test/Tools/circt-bmc/externalize-registers.mlir @@ -133,12 +133,12 @@ hw.module @named_firregs(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out ou hw.output %secondreg : i32 } -// CHECK: hw.module @firreg_with_reset(in [[CLK:%.+]] : !seq.clock, in [[RST:%.+]] : i1, in [[IN:%.+]] : i32, in [[OLD_REG:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit], num_regs = 1 : i32} { +// CHECK: hw.module @firreg_with_sync_reset(in [[CLK:%.+]] : !seq.clock, in [[RST:%.+]] : i1, in [[IN:%.+]] : i32, in [[OLD_REG1:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit], num_regs = 1 : i32} { // CHECK: [[C0_I32:%.+]] = hw.constant 0 : i32 -// CHECK: [[MUX:%.+]] = comb.mux [[RST]], [[C0_I32]], [[IN]] : i32 -// CHECK: hw.output [[OLD_REG]], [[MUX]] +// CHECK: [[MUX1:%.+]] = comb.mux [[RST]], [[C0_I32]], [[IN]] : i32 +// CHECK: hw.output [[OLD_REG1]], [[MUX1]] // CHECK: } -hw.module @firreg_with_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out: i32) { +hw.module @firreg_with_sync_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out: i32) { %c0_i32 = hw.constant 0 : i32 %1 = seq.firreg %in clock %clk reset sync %rst, %c0_i32 : i32 hw.output %1 : i32 From d7f66a951be73cf5c6ec8cc4c6c565568cb93e38 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Sat, 12 Jul 2025 18:54:41 +0800 Subject: [PATCH 03/11] pass clang-tidy --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index d78c454b8ef4..2bdb11415e6a 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -195,10 +195,10 @@ void ExternalizeRegistersPass::runOnOperation() { // Async reset regOp.emitError("seq.firreg with async reset not yet supported"); return signalPassFailure(); - } else { - // Sync reset - regOp.getResult().replaceAllUsesWith(newInput); } + // Sync reset + regOp.getResult().replaceAllUsesWith(newInput); + auto mux = builder.create(regOp.getLoc(), regOp.getType(), reset, resetValue, regOp.getNext()); From ce8899e8e0201b1ef836bc68a261bdf851901cb5 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Wed, 16 Jul 2025 18:42:09 +0800 Subject: [PATCH 04/11] fix format and typo in test --- test/Tools/circt-bmc/externalize-registers-errors.mlir | 2 +- test/Tools/circt-bmc/externalize-registers.mlir | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Tools/circt-bmc/externalize-registers-errors.mlir b/test/Tools/circt-bmc/externalize-registers-errors.mlir index d57891a0e8c4..86f47202e0c8 100644 --- a/test/Tools/circt-bmc/externalize-registers-errors.mlir +++ b/test/Tools/circt-bmc/externalize-registers-errors.mlir @@ -61,4 +61,4 @@ hw.module @firreg_with_async_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32 // expected-error @below {{seq.firreg with async reset not yet supported}} %1 = seq.firreg %in clock %clk reset async %rst, %c0_i32 : i32 hw.output %1 : i32 -} \ No newline at end of file +} diff --git a/test/Tools/circt-bmc/externalize-registers.mlir b/test/Tools/circt-bmc/externalize-registers.mlir index 88b6e8b5cc7b..6586c3bde194 100644 --- a/test/Tools/circt-bmc/externalize-registers.mlir +++ b/test/Tools/circt-bmc/externalize-registers.mlir @@ -111,11 +111,11 @@ hw.module @one_firreg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: hw.output %single_reg : i32 } -// CHECK: hw.module @combreg_and_firreg(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in [[OLD_REG0:%.+]] : i32, in [[OLD_REG1:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit, unit], num_regs = 2 : i32} { +// CHECK: hw.module @compreg_and_firreg(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in [[OLD_REG0:%.+]] : i32, in [[OLD_REG1:%.+]] : i32, out {{.+}} : i32, out {{.+}} : i32, out {{.+}} : i32) attributes {initial_values = [unit, unit], num_regs = 2 : i32} { // CHECK: [[ADD:%.+]] = comb.add [[IN0]], [[IN1]] // CHECK: hw.output [[OLD_REG1]], [[ADD]], [[OLD_REG0]] // CHECK: } -hw.module @combreg_and_firreg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: i32) { +hw.module @compreg_and_firreg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: i32) { %0 = comb.add %in0, %in1 : i32 %1 = seq.compreg %0, %clk : i32 %2 = seq.firreg %1 clock %clk : i32 From 94b2b3b65cae98421dd37ff8b484ab37b6b7c6a0 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Thu, 17 Jul 2025 13:55:20 +0800 Subject: [PATCH 05/11] extract function `externalizeReg` --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 169 ++++++++---------- .../externalize-registers-errors.mlir | 2 +- 2 files changed, 77 insertions(+), 94 deletions(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 2bdb11415e6a..2f0df86a8348 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -43,18 +43,24 @@ struct ExternalizeRegistersPass : public circt::impl::ExternalizeRegistersBase { using ExternalizeRegistersBase::ExternalizeRegistersBase; void runOnOperation() override; -}; -} // namespace -void ExternalizeRegistersPass::runOnOperation() { - auto &instanceGraph = getAnalysis(); - DenseSet handled; +private: DenseMap> addedInputs; DenseMap> addedInputNames; DenseMap> addedOutputs; DenseMap> addedOutputNames; DenseMap> initialValues; + LogicalResult externalizeReg(HWModuleOp module, Operation *op, Twine regName, + Value clock, Attribute initState, Value reset, + bool isAsync, Value resetValue, Value next); +}; +} // namespace + +void ExternalizeRegistersPass::runOnOperation() { + auto &instanceGraph = getAnalysis(); + DenseSet handled; + // Iterate over all instances in the instance graph. This ensures we visit // every module, even private top modules (private and never instantiated). for (auto *startNode : instanceGraph) { @@ -86,12 +92,7 @@ void ExternalizeRegistersPass::runOnOperation() { } module->walk([&](Operation *op) { if (auto regOp = dyn_cast(op)) { - if (!isa(regOp.getClk())) { - regOp.emitError("only clocks directly given as block arguments " - "are supported"); - return signalPassFailure(); - } - mlir::Attribute initState; + mlir::Attribute initState = {}; if (auto initVal = regOp.getInitialValue()) { // Find the constant op that defines the reset value in an initial // block (if it exists) @@ -111,104 +112,37 @@ void ExternalizeRegistersPass::runOnOperation() { "not yet supported"); return signalPassFailure(); } - } else { - // If there's no initial value just add a unit attribute to maintain - // one-to-one correspondence with module ports - initState = mlir::UnitAttr::get(&getContext()); } - addedInputs[module.getSymNameAttr()].push_back(regOp.getType()); - addedOutputs[module.getSymNameAttr()].push_back( - regOp.getInput().getType()); - OpBuilder builder(regOp); - auto regName = regOp.getName(); - StringAttr newInputName, newOutputName; - if (regName && !regName.value().empty()) { - newInputName = builder.getStringAttr(regName.value() + "_state"); - newOutputName = builder.getStringAttr(regName.value() + "_input"); - } else { - newInputName = - builder.getStringAttr("reg_" + Twine(numRegs) + "_state"); - newOutputName = - builder.getStringAttr("reg_" + Twine(numRegs) + "_input"); - } - addedInputNames[module.getSymNameAttr()].push_back(newInputName); - addedOutputNames[module.getSymNameAttr()].push_back(newOutputName); - initialValues[module.getSymNameAttr()].push_back(initState); + Twine regName = regOp.getName() && !(regOp.getName().value().empty()) + ? regOp.getName().value() + : "reg_" + Twine(numRegs); - regOp.getResult().replaceAllUsesWith( - module.appendInput(newInputName, regOp.getType()).second); - if (auto reset = regOp.getReset()) { - auto resetValue = regOp.getResetValue(); - auto mux = builder.create( - regOp.getLoc(), regOp.getType(), reset, resetValue, - regOp.getInput()); - module.appendOutput(newOutputName, mux); - } else { - module.appendOutput(newOutputName, regOp.getInput()); + if (failed(externalizeReg(module, op, regName, regOp.getClk(), + initState, regOp.getReset(), false, + regOp.getResetValue(), regOp.getInput()))) { + return signalPassFailure(); } regOp->erase(); ++numRegs; return; } if (auto regOp = dyn_cast(op)) { - if (!isa(regOp.getClk())) { - regOp.emitError("only clocks directly given as block arguments " - "are supported"); - return signalPassFailure(); - } - mlir::Attribute initState; + mlir::Attribute initState = {}; if (auto preset = regOp.getPreset()) { // Get preset value as initState initState = mlir::IntegerAttr::get( mlir::IntegerType::get(&getContext(), preset->getBitWidth()), *preset); - } else { - // If there's no preset value just add a unit attribute to maintain - // one-to-one correspondence with module ports - initState = mlir::UnitAttr::get(&getContext()); } - addedInputs[module.getSymNameAttr()].push_back(regOp.getType()); - addedOutputs[module.getSymNameAttr()].push_back( - regOp.getNext().getType()); - OpBuilder builder(regOp); - auto regName = regOp.getName(); - StringAttr newInputName, newOutputName; - if (!regName.empty()) { - newInputName = builder.getStringAttr(regName + "_state"); - newOutputName = builder.getStringAttr(regName + "_input"); - } else { - newInputName = - builder.getStringAttr("reg_" + Twine(numRegs) + "_state"); - newOutputName = - builder.getStringAttr("reg_" + Twine(numRegs) + "_input"); - } - addedInputNames[module.getSymNameAttr()].push_back(newInputName); - addedOutputNames[module.getSymNameAttr()].push_back(newOutputName); - initialValues[module.getSymNameAttr()].push_back(initState); + Twine regName = regOp.getName().empty() ? "reg_" + Twine(numRegs) + : regOp.getName(); - // Replace the register with newInput and newOutput - auto newInput = - module.appendInput(newInputName, regOp.getType()).second; - if (auto reset = regOp.getReset()) { - auto resetValue = regOp.getResetValue(); - if (regOp.getIsAsync()) { - // Async reset - regOp.emitError("seq.firreg with async reset not yet supported"); - return signalPassFailure(); - } - // Sync reset - regOp.getResult().replaceAllUsesWith(newInput); - - auto mux = - builder.create(regOp.getLoc(), regOp.getType(), - reset, resetValue, regOp.getNext()); - module.appendOutput(newOutputName, mux); - } else { - // No reset - regOp.getResult().replaceAllUsesWith(newInput); - module.appendOutput(newOutputName, regOp.getNext()); + if (failed(externalizeReg(module, op, regName, regOp.getClk(), + initState, regOp.getReset(), + regOp.getIsAsync(), regOp.getResetValue(), + regOp.getNext()))) { + return signalPassFailure(); } - regOp->erase(); ++numRegs; return; @@ -273,3 +207,52 @@ void ExternalizeRegistersPass::runOnOperation() { } } } + +LogicalResult ExternalizeRegistersPass::externalizeReg( + HWModuleOp module, Operation *op, Twine regName, Value clock, + Attribute initState, Value reset, bool isAsync, Value resetValue, + Value next) { + if (!isa(clock)) { + op->emitError("only clocks directly given as block arguments " + "are supported"); + return failure(); + } + + OpBuilder builder(op); + auto result = op->getResult(0); + auto regType = result.getType(); + + // If there's no initial value just add a unit attribute to maintain + // one-to-one correspondence with module ports + initialValues[module.getSymNameAttr()].push_back( + initState ? initState : mlir::UnitAttr::get(&getContext())); + + StringAttr newInputName(builder.getStringAttr(regName + "_state")), + newOutputName(builder.getStringAttr(regName + "_input")); + addedInputs[module.getSymNameAttr()].push_back(regType); + addedInputNames[module.getSymNameAttr()].push_back(newInputName); + addedOutputs[module.getSymNameAttr()].push_back(next.getType()); + addedOutputNames[module.getSymNameAttr()].push_back(newOutputName); + + // Replace the register with newInput and newOutput + auto newInput = module.appendInput(newInputName, regType).second; + if (reset) { + if (isAsync) { + // Async reset + op->emitError("registors with async reset not yet supported"); + return failure(); + } + // Sync reset + result.replaceAllUsesWith(newInput); + + auto mux = builder.create(op->getLoc(), regType, reset, + resetValue, next); + module.appendOutput(newOutputName, mux); + } else { + // No reset + result.replaceAllUsesWith(newInput); + module.appendOutput(newOutputName, next); + } + + return success(); +} diff --git a/test/Tools/circt-bmc/externalize-registers-errors.mlir b/test/Tools/circt-bmc/externalize-registers-errors.mlir index 86f47202e0c8..0883b6727c78 100644 --- a/test/Tools/circt-bmc/externalize-registers-errors.mlir +++ b/test/Tools/circt-bmc/externalize-registers-errors.mlir @@ -58,7 +58,7 @@ hw.module @reg_with_instance_initial(in %clk: !seq.clock, in %in: i32, out out: // ----- hw.module @firreg_with_async_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out: i32) { %c0_i32 = hw.constant 0 : i32 - // expected-error @below {{seq.firreg with async reset not yet supported}} + // expected-error @below {{registors with async reset not yet supported}} %1 = seq.firreg %in clock %clk reset async %rst, %c0_i32 : i32 hw.output %1 : i32 } From 7c3bb40506662294d8945c77e26ca3685ee4400e Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Thu, 17 Jul 2025 14:02:44 +0800 Subject: [PATCH 06/11] rename "xxx_input" to "xxx_next" --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 2 +- test/Tools/circt-bmc/externalize-registers.mlir | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 2f0df86a8348..27ed20b044fa 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -228,7 +228,7 @@ LogicalResult ExternalizeRegistersPass::externalizeReg( initState ? initState : mlir::UnitAttr::get(&getContext())); StringAttr newInputName(builder.getStringAttr(regName + "_state")), - newOutputName(builder.getStringAttr(regName + "_input")); + newOutputName(builder.getStringAttr(regName + "_next")); addedInputs[module.getSymNameAttr()].push_back(regType); addedInputNames[module.getSymNameAttr()].push_back(newInputName); addedOutputs[module.getSymNameAttr()].push_back(next.getType()); diff --git a/test/Tools/circt-bmc/externalize-registers.mlir b/test/Tools/circt-bmc/externalize-registers.mlir index 6586c3bde194..445edbfed2c6 100644 --- a/test/Tools/circt-bmc/externalize-registers.mlir +++ b/test/Tools/circt-bmc/externalize-registers.mlir @@ -58,8 +58,8 @@ hw.module @nested_reg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: hw.output %0 : i32 } -// CHECK: hw.module @nested_nested_reg(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in %single_reg_state : i32, in %top_reg_state : i32, out {{.+}} : i32, out single_reg_input : i32, out top_reg_input : i32) attributes {initial_values = [0 : i32, unit], num_regs = 2 : i32} { -// CHECK: [[INSTOUT:%.+]], [[INSTREG:%.+]] = hw.instance "nested_reg" @nested_reg(clk: [[CLK]]: !seq.clock, in0: [[IN0]]: i32, in1: [[IN1]]: i32, single_reg_state: %single_reg_state: i32) -> ({{.+}}: i32, single_reg_input: i32) +// CHECK: hw.module @nested_nested_reg(in [[CLK:%.+]] : !seq.clock, in [[IN0:%.+]] : i32, in [[IN1:%.+]] : i32, in %single_reg_state : i32, in %top_reg_state : i32, out {{.+}} : i32, out single_reg_next : i32, out top_reg_next : i32) attributes {initial_values = [0 : i32, unit], num_regs = 2 : i32} { +// CHECK: [[INSTOUT:%.+]], [[INSTREG:%.+]] = hw.instance "nested_reg" @nested_reg(clk: [[CLK]]: !seq.clock, in0: [[IN0]]: i32, in1: [[IN1]]: i32, single_reg_state: %single_reg_state: i32) -> ({{.+}}: i32, single_reg_next: i32) // CHECK: hw.output %top_reg_state, [[INSTREG]], [[INSTOUT]] // CHECK: } hw.module @nested_nested_reg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, out out: i32) { @@ -68,7 +68,7 @@ hw.module @nested_nested_reg(in %clk: !seq.clock, in %in0: i32, in %in1: i32, ou hw.output %top_reg : i32 } -// CHECK: hw.module @different_initial_values(in [[CLK:%.+]] : !seq.clock, in [[IN:%.+]] : i32, in %reg0_state : i32, in %reg1_state : i32, in %reg2_state : i32, out reg0_input : i32, out reg1_input : i32, out reg2_input : i32) attributes {initial_values = [0 : i32, 42 : i32, unit], num_regs = 3 : i32} { +// CHECK: hw.module @different_initial_values(in [[CLK:%.+]] : !seq.clock, in [[IN:%.+]] : i32, in %reg0_state : i32, in %reg1_state : i32, in %reg2_state : i32, out reg0_next : i32, out reg1_next : i32, out reg2_next : i32) attributes {initial_values = [0 : i32, 42 : i32, unit], num_regs = 3 : i32} { // CHECK: [[INITIAL:%.+]]:2 = seq.initial() { // CHECK: [[C0_I32:%.+]] = hw.constant 0 : i32 // CHECK: [[C42_I32:%.+]] = hw.constant 42 : i32 From c4f22bc921a85c40d4873dc3951c350a041b660e Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Fri, 18 Jul 2025 17:03:12 +0800 Subject: [PATCH 07/11] Update lib/Tools/circt-bmc/ExternalizeRegisters.cpp Co-authored-by: Bea Healy <57840981+TaoBi22@users.noreply.github.com> --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 27ed20b044fa..1d523f68fb90 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -239,7 +239,7 @@ LogicalResult ExternalizeRegistersPass::externalizeReg( if (reset) { if (isAsync) { // Async reset - op->emitError("registors with async reset not yet supported"); + op->emitError("registers with an async reset are not yet supported"); return failure(); } // Sync reset From d9992ea1d30ac1798282827107a84473f5eb5135 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Fri, 18 Jul 2025 17:03:26 +0800 Subject: [PATCH 08/11] Update test/Tools/circt-bmc/externalize-registers-errors.mlir Co-authored-by: Bea Healy <57840981+TaoBi22@users.noreply.github.com> --- test/Tools/circt-bmc/externalize-registers-errors.mlir | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Tools/circt-bmc/externalize-registers-errors.mlir b/test/Tools/circt-bmc/externalize-registers-errors.mlir index 0883b6727c78..4941ad2de6f0 100644 --- a/test/Tools/circt-bmc/externalize-registers-errors.mlir +++ b/test/Tools/circt-bmc/externalize-registers-errors.mlir @@ -58,7 +58,7 @@ hw.module @reg_with_instance_initial(in %clk: !seq.clock, in %in: i32, out out: // ----- hw.module @firreg_with_async_reset(in %clk: !seq.clock, in %rst: i1, in %in: i32, out out: i32) { %c0_i32 = hw.constant 0 : i32 - // expected-error @below {{registors with async reset not yet supported}} + // expected-error @below {{registers with an async reset are not yet supported}} %1 = seq.firreg %in clock %clk reset async %rst, %c0_i32 : i32 hw.output %1 : i32 } From 21e14cb5b6443e37270617416c2a759ad94e277e Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Fri, 18 Jul 2025 16:48:59 +0800 Subject: [PATCH 09/11] add if-else for sync/async reset --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 1d523f68fb90..30bc61a6dff2 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -237,17 +237,17 @@ LogicalResult ExternalizeRegistersPass::externalizeReg( // Replace the register with newInput and newOutput auto newInput = module.appendInput(newInputName, regType).second; if (reset) { - if (isAsync) { + if (!isAsync) { + // Sync reset + result.replaceAllUsesWith(newInput); + auto mux = builder.create(op->getLoc(), regType, reset, + resetValue, next); + module.appendOutput(newOutputName, mux); + } else { // Async reset op->emitError("registers with an async reset are not yet supported"); return failure(); } - // Sync reset - result.replaceAllUsesWith(newInput); - - auto mux = builder.create(op->getLoc(), regType, reset, - resetValue, next); - module.appendOutput(newOutputName, mux); } else { // No reset result.replaceAllUsesWith(newInput); From 1ba03e9769b660223f97bdaa4d8cd976235d3561 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Sun, 20 Jul 2025 16:41:52 +0800 Subject: [PATCH 10/11] Revert "add if-else for sync/async reset" This reverts commit 21e14cb5b6443e37270617416c2a759ad94e277e. --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 30bc61a6dff2..1d523f68fb90 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -237,17 +237,17 @@ LogicalResult ExternalizeRegistersPass::externalizeReg( // Replace the register with newInput and newOutput auto newInput = module.appendInput(newInputName, regType).second; if (reset) { - if (!isAsync) { - // Sync reset - result.replaceAllUsesWith(newInput); - auto mux = builder.create(op->getLoc(), regType, reset, - resetValue, next); - module.appendOutput(newOutputName, mux); - } else { + if (isAsync) { // Async reset op->emitError("registers with an async reset are not yet supported"); return failure(); } + // Sync reset + result.replaceAllUsesWith(newInput); + + auto mux = builder.create(op->getLoc(), regType, reset, + resetValue, next); + module.appendOutput(newOutputName, mux); } else { // No reset result.replaceAllUsesWith(newInput); From ec097607f3098fc12cdc9acf390b1346518c0484 Mon Sep 17 00:00:00 2001 From: Yicheng Liu Date: Sun, 20 Jul 2025 16:50:20 +0800 Subject: [PATCH 11/11] move `replaceAllUsesWith` above the if-else --- lib/Tools/circt-bmc/ExternalizeRegisters.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp index 1d523f68fb90..192cc560abed 100644 --- a/lib/Tools/circt-bmc/ExternalizeRegisters.cpp +++ b/lib/Tools/circt-bmc/ExternalizeRegisters.cpp @@ -236,6 +236,7 @@ LogicalResult ExternalizeRegistersPass::externalizeReg( // Replace the register with newInput and newOutput auto newInput = module.appendInput(newInputName, regType).second; + result.replaceAllUsesWith(newInput); if (reset) { if (isAsync) { // Async reset @@ -243,14 +244,11 @@ LogicalResult ExternalizeRegistersPass::externalizeReg( return failure(); } // Sync reset - result.replaceAllUsesWith(newInput); - auto mux = builder.create(op->getLoc(), regType, reset, resetValue, next); module.appendOutput(newOutputName, mux); } else { // No reset - result.replaceAllUsesWith(newInput); module.appendOutput(newOutputName, next); }