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..ea6c264f6b6d 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 : Pass<"firrtl-check-uninferred-resets", "firrtl::CircuitOp"> { + 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..ea6cc33f6dfa --- /dev/null +++ b/lib/Dialect/FIRRTL/Transforms/CheckUninferredResets.cpp @@ -0,0 +1,62 @@ +//===- 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; + + LogicalResult verifyNoAbstractReset(); +}; +} // namespace + +void CheckUninferredResetsPass::runOnOperation() { + 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 signalPassFailure(); +} + +std::unique_ptr circt::firrtl::createCheckUninferredResetsPass() { + return std::make_unique(); +} \ No newline at end of file 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..e186031e085a 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().addPass( + 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..a74ae9866234 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,firrtl-check-uninferred-resets))' --verify-diagnostics --split-input-file %s // Tests extracted from: // - github.com/chipsalliance/firrtl: 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