diff --git a/include/circt/Dialect/FIRRTL/Passes.h b/include/circt/Dialect/FIRRTL/Passes.h index d3dd11735179..1b4ea15cf75f 100644 --- a/include/circt/Dialect/FIRRTL/Passes.h +++ b/include/circt/Dialect/FIRRTL/Passes.h @@ -214,6 +214,8 @@ std::unique_ptr createLowerDPIPass(); std::unique_ptr createAssignOutputDirsPass(mlir::StringRef outputDir = ""); +std::unique_ptr createCheckUninferredResetsPass(); + /// Generate the code for registering passes. #define GEN_PASS_REGISTRATION #include "circt/Dialect/FIRRTL/Passes.h.inc" diff --git a/include/circt/Dialect/FIRRTL/Passes.td b/include/circt/Dialect/FIRRTL/Passes.td index 3c3e3c4d841a..45af58adfad0 100644 --- a/include/circt/Dialect/FIRRTL/Passes.td +++ b/include/circt/Dialect/FIRRTL/Passes.td @@ -970,4 +970,16 @@ def ProbesToSignals : Pass<"firrtl-probes-to-signals", "firrtl::CircuitOp"> { let constructor = "circt::firrtl::createProbesToSignalsPass()"; } +def CheckUninferredResets : InterfacePass<"firrtl-check-uninferred-resets", "firrtl::FModuleLike"> { + let summary = "Verify no uninferred resets"; + let description = [{ + This pass checks to see if there are abstract type resets which have not + been inferred correctly. + + Run after Inliner since the reset type of an unreferenced module (which + doesn't have a parent module) cannot be inferred. + }]; + let constructor = "circt::firrtl::createCheckUninferredResetsPass()"; +} + #endif // CIRCT_DIALECT_FIRRTL_PASSES_TD diff --git a/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt b/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt index c885ea49e910..f59174e680bf 100755 --- a/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt +++ b/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt @@ -3,6 +3,7 @@ add_circt_dialect_library(CIRCTFIRRTLTransforms AddSeqMemPorts.cpp BlackBoxReader.cpp CheckCombLoops.cpp + CheckUninferredResets.cpp CreateCompanionAssume.cpp CreateSiFiveMetadata.cpp Dedup.cpp diff --git a/lib/Dialect/FIRRTL/Transforms/CheckUninferredResets.cpp b/lib/Dialect/FIRRTL/Transforms/CheckUninferredResets.cpp new file mode 100644 index 000000000000..3ea303523213 --- /dev/null +++ b/lib/Dialect/FIRRTL/Transforms/CheckUninferredResets.cpp @@ -0,0 +1,55 @@ +//===- CheckUninferredResets.cpp - Verify no uninferred resets ------===// +// +// 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 +// +//===----------------------------------------------------------------------===// +// +// This pass checks to see if there are abstract type resets which have not +// been inferred correctly. +// +//===----------------------------------------------------------------------===// + +#include "circt/Dialect/FIRRTL/FIRRTLOpInterfaces.h" +#include "circt/Dialect/FIRRTL/FIRRTLUtils.h" +#include "circt/Dialect/FIRRTL/Passes.h" +#include "mlir/Pass/Pass.h" + +namespace circt { +namespace firrtl { +#define GEN_PASS_DEF_CHECKUNINFERREDRESETS +#include "circt/Dialect/FIRRTL/Passes.h.inc" +} // namespace firrtl +} // namespace circt + +using namespace mlir; +using namespace circt; +using namespace firrtl; + +namespace { +struct CheckUninferredResetsPass + : public circt::firrtl::impl::CheckUninferredResetsBase< + CheckUninferredResetsPass> { + void runOnOperation() override; +}; +} // namespace + +void CheckUninferredResetsPass::runOnOperation() { + auto module = getOperation(); + for (auto port : module.getPorts()) { + if (getBaseOfType(port.type)) { + auto diag = emitError(port.loc) + << "a port \"" << port.getName() + << "\" with abstract reset type was unable to be " + "inferred by InferResets (is this a top-level port?)"; + diag.attachNote(module->getLoc()) + << "the module with this uninferred reset port was defined here"; + return signalPassFailure(); + } + } +} + +std::unique_ptr circt::firrtl::createCheckUninferredResetsPass() { + return std::make_unique(); +} diff --git a/lib/Dialect/FIRRTL/Transforms/InferResets.cpp b/lib/Dialect/FIRRTL/Transforms/InferResets.cpp index ca52ee6c470a..b7da01f89678 100644 --- a/lib/Dialect/FIRRTL/Transforms/InferResets.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InferResets.cpp @@ -461,8 +461,6 @@ struct InferResetsPass LogicalResult implementFullReset(FModuleOp module, ResetDomain &domain); void implementFullReset(Operation *op, FModuleOp module, Value actualReset); - LogicalResult verifyNoAbstractReset(); - //===--------------------------------------------------------------------===// // Utilities @@ -546,10 +544,6 @@ void InferResetsPass::runOnOperationInner() { // Implement the full resets. if (failed(implementFullReset())) return signalPassFailure(); - - // Require that no Abstract Resets exist on ports in the design. - if (failed(verifyNoAbstractReset())) - return signalPassFailure(); } std::unique_ptr circt::firrtl::createInferResetsPass() { @@ -1909,25 +1903,3 @@ void InferResetsPass::implementFullReset(Operation *op, FModuleOp module, regOp.getResetValueMutable().assign(zero); } } - -LogicalResult InferResetsPass::verifyNoAbstractReset() { - bool hasAbstractResetPorts = false; - for (FModuleLike module : - getOperation().getBodyBlock()->getOps()) { - for (PortInfo port : module.getPorts()) { - if (getBaseOfType(port.type)) { - auto diag = emitError(port.loc) - << "a port \"" << port.getName() - << "\" with abstract reset type was unable to be " - "inferred by InferResets (is this a top-level port?)"; - diag.attachNote(module->getLoc()) - << "the module with this uninferred reset port was defined here"; - hasAbstractResetPorts = true; - } - } - } - - if (hasAbstractResetPorts) - return failure(); - return success(); -} diff --git a/lib/Firtool/Firtool.cpp b/lib/Firtool/Firtool.cpp index 07d93548de3b..1ef3f25ad3ce 100644 --- a/lib/Firtool/Firtool.cpp +++ b/lib/Firtool/Firtool.cpp @@ -142,6 +142,9 @@ LogicalResult firtool::populateCHIRRTLToLowFIRRTL(mlir::PassManager &pm, pm.nest().addPass(firrtl::createInlinerPass()); + pm.nest().addNestedPass( + firrtl::createCheckUninferredResetsPass()); + // Preset the random initialization parameters for each module. The current // implementation assumes it can run at a time where every register is // currently in the final module it will be emitted in, all registers have diff --git a/test/Dialect/FIRRTL/infer-resets-errors.mlir b/test/Dialect/FIRRTL/infer-resets-errors.mlir index 15c45ef61f18..2dfc9a917796 100644 --- a/test/Dialect/FIRRTL/infer-resets-errors.mlir +++ b/test/Dialect/FIRRTL/infer-resets-errors.mlir @@ -1,4 +1,4 @@ -// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets))' --verify-diagnostics --split-input-file %s +// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets,any(firrtl-check-uninferred-resets)))' --verify-diagnostics --split-input-file %s // Tests extracted from: // - github.com/chipsalliance/firrtl: diff --git a/test/Dialect/FIRRTL/legal-uninferred-resets.mlir b/test/Dialect/FIRRTL/legal-uninferred-resets.mlir new file mode 100644 index 000000000000..348f618046bb --- /dev/null +++ b/test/Dialect/FIRRTL/legal-uninferred-resets.mlir @@ -0,0 +1,21 @@ +// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets,any(firrtl-check-uninferred-resets)))' --verify-diagnostics %s +// RUN: circt-opt --pass-pipeline='builtin.module(firrtl.circuit(firrtl-infer-resets,firrtl-inliner,any(firrtl-check-uninferred-resets)))' %s | FileCheck %s + +firrtl.circuit "LegalUninferredReset" { + // The following two diagnostics will only appear if firrtl-inliner is not enabled + // expected-note @+2 {{the module with this uninferred reset port was defined here}} + // expected-error @+1 {{a port "reset" with abstract reset type was unable to be inferred by InferResets}} + firrtl.module private @Adder(in %clock: !firrtl.clock, in %reset: !firrtl.reset, in %in: !firrtl.uint<10>, out %out: !firrtl.uint<10>) { + %c1_ui1 = firrtl.constant 1 : !firrtl.const.uint<1> + %0 = firrtl.add %in, %c1_ui1 : (!firrtl.uint<10>, !firrtl.const.uint<1>) -> !firrtl.uint<11> + %_out_T = firrtl.node interesting_name %0 : !firrtl.uint<11> + %1 = firrtl.tail %_out_T, 1 : (!firrtl.uint<11>) -> !firrtl.uint<10> + %_out_T_1 = firrtl.node interesting_name %1 : !firrtl.uint<10> + firrtl.matchingconnect %out, %_out_T_1 : !firrtl.uint<10> + } + firrtl.module @LegalUninferredReset(in %clock: !firrtl.clock, in %reset: !firrtl.uint<1>) { + } +} + +// CHECK-NOT: firrtl.circuit "Adder" +// CHECK: firrtl.circuit "LegalUninferredReset" \ No newline at end of file diff --git a/test/firtool/legal-uninferred-resets.fir b/test/firtool/legal-uninferred-resets.fir new file mode 100644 index 000000000000..d30de95e0fe1 --- /dev/null +++ b/test/firtool/legal-uninferred-resets.fir @@ -0,0 +1,31 @@ +; RUN: firtool %s --verify-diagnostics --split-input-file +; The Chisel's Definition API generates modules that are not instantiated, +; whose reset cannot be inferred properly. These modules should be removed +; before checking abstract type resets. + +FIRRTL version 3.3.0 +circuit Foo: + module Adder : + input clock : Clock + input reset : Reset + input in : UInt<10> + output out : UInt<10> + + node _out_T = add(in, UInt<1>(0h1)) + node _out_T_1 = tail(_out_T, 1) + connect out, _out_T_1 + + ; expected-warning @below {{module `Foo` is empty}} + module Foo : + input clock : Clock + input reset : UInt<1> + +;// ----- + +FIRRTL version 3.3.0 +circuit Bar: + ; expected-note @below {{the module with this uninferred reset port was defined here}} + module Bar : + input clock : Clock + ; expected-error @below {{a port "reset" with abstract reset type was unable to be inferred by InferResets}} + input reset : Reset