diff --git a/include/circt/Dialect/FIRRTL/Passes.h b/include/circt/Dialect/FIRRTL/Passes.h index d3dd11735179..1ae1e07e5358 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 createCheckRecursiveInstantiation(); + /// 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 4af7367449b7..22c2207f47ad 100644 --- a/include/circt/Dialect/FIRRTL/Passes.td +++ b/include/circt/Dialect/FIRRTL/Passes.td @@ -968,4 +968,18 @@ def ProbesToSignals : Pass<"firrtl-probes-to-signals", "firrtl::CircuitOp"> { let constructor = "circt::firrtl::createProbesToSignalsPass()"; } +def CheckRecursiveInstantiation : Pass<"firrtl-check-recursive-instantiation", + "firrtl::CircuitOp"> { + let summary = "Check for illegal recursive instantiation"; + let description = [{ + This pass checks for illegal recursive module instantion. Recursive + instantiation is when a module instantiates itself, either directly or + indirectly through other modules it instantiates. Recursive module + instantiation is illegal because it would require infinite hardware to + synthesize. Recursive class instantiation is illegal as it would create an + infinite loop. + }]; + let constructor = "circt::firrtl::createCheckRecursiveInstantiation()"; +} + #endif // CIRCT_DIALECT_FIRRTL_PASSES_TD diff --git a/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt b/lib/Dialect/FIRRTL/Transforms/CMakeLists.txt index c885ea49e910..e86980a540a3 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 + CheckRecursiveInstantiation.cpp CreateCompanionAssume.cpp CreateSiFiveMetadata.cpp Dedup.cpp diff --git a/lib/Dialect/FIRRTL/Transforms/CheckRecursiveInstantiation.cpp b/lib/Dialect/FIRRTL/Transforms/CheckRecursiveInstantiation.cpp new file mode 100644 index 000000000000..59c44fd11f8b --- /dev/null +++ b/lib/Dialect/FIRRTL/Transforms/CheckRecursiveInstantiation.cpp @@ -0,0 +1,96 @@ +//===- CheckRecursiveInstantiation.cpp - Check recurisve instantiation ----===// +// +// 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 "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" +#include "circt/Dialect/FIRRTL/Passes.h" +#include "mlir/Pass/Pass.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SCCIterator.h" + +namespace circt { +namespace firrtl { +#define GEN_PASS_DEF_CHECKRECURSIVEINSTANTIATION +#include "circt/Dialect/FIRRTL/Passes.h.inc" +} // namespace firrtl +} // namespace circt + +using namespace circt; +using namespace firrtl; + +static void printPath(InstanceGraph &instanceGraph, + ArrayRef nodes) { + assert(nodes.size() > 0 && "an scc should have at least one node"); + + // The first node in the SCC where we start the path from. + auto *first = nodes.front(); + auto diag = emitError(first->getModule().getLoc(), "recursive instantiation"); + + // A set of all nodes in the SCC for quick lookup. + llvm::SmallPtrSet scc(nodes.begin(), nodes.end()); + + // A set of all nodes seen in the path. + llvm::SmallPtrSet visited; + + // The current node we are examining. + auto *current = first; + while (true) { + // Iterate all instances in this module. + for (auto *record : *current) { + auto *target = record->getTarget(); + // Filter out the edges that don't point in the SCC. + if (!scc.contains(target)) + continue; + // If this node is already on our path, skip it. + if (visited.contains(target)) + continue; + auto ¬e = diag.attachNote(record->getInstance().getLoc()); + note << record->getParent()->getModule().getModuleName(); + note << " instantiates " + << record->getTarget()->getModule().getModuleName() << " here"; + // If we have looped back to the starting point, return here. + if (target == first) + return; + // Recurse on this edge. + current = target; + visited.insert(target); + break; + } + } +} + +static LogicalResult run(InstanceGraph &instanceGraph) { + bool error = false; + for (auto it = llvm::scc_begin(&instanceGraph), + end = llvm::scc_end(&instanceGraph); + it != end; ++it) { + if (it.hasCycle()) { + printPath(instanceGraph, *it); + error = true; + } + } + return failure(error); +} + +namespace { + +class CheckRecursiveInstantiationPass + : public impl::CheckRecursiveInstantiationBase< + CheckRecursiveInstantiationPass> { +public: + void runOnOperation() override { + auto &instanceGraph = getAnalysis(); + if (failed(run(instanceGraph))) + return signalPassFailure(); + markAllAnalysesPreserved(); + } // namespace +}; +} // namespace + +std::unique_ptr circt::firrtl::createCheckRecursiveInstantiation() { + return std::make_unique(); +} diff --git a/test/Dialect/FIRRTL/check-recursive-instantiation-errors.mlir b/test/Dialect/FIRRTL/check-recursive-instantiation-errors.mlir new file mode 100644 index 000000000000..4f85c7a59820 --- /dev/null +++ b/test/Dialect/FIRRTL/check-recursive-instantiation-errors.mlir @@ -0,0 +1,70 @@ +// RUN: circt-opt -pass-pipeline='builtin.module(firrtl.circuit(firrtl-check-recursive-instantiation))' %s --verify-diagnostics --split-input-file + +firrtl.circuit "SelfLoop0" { + // expected-error @below {{recursive instantiation}} + firrtl.module @SelfLoop0() { + // expected-note @below {{SelfLoop0 instantiates SelfLoop0 here}} + firrtl.instance inst @SelfLoop0() + } +} + +// ----- + +firrtl.circuit "SelfLoop1" { + // expected-error @below {{recursive instantiation}} + firrtl.module @SelfLoop1() { + // Should only print one of the instance paths. + // expected-note @below {{SelfLoop1 instantiates SelfLoop1 here}} + firrtl.instance inst @SelfLoop1() + firrtl.instance inst @SelfLoop1() + } +} + +// ----- + +firrtl.circuit "TwoLoops" { + // Should report both recursive module instantions. + // expected-error @below {{recursive instantiation}} + firrtl.module @TwoLoops() { + // expected-note @below {{TwoLoops instantiates TwoLoops here}} + firrtl.instance inst @TwoLoops() + firrtl.instance inst @OtherModule() + } + // expected-error @below {{recursive instantiation}} + firrtl.module @OtherModule() { + // expected-note @below {{OtherModule instantiates OtherModule here}} + firrtl.instance inst @OtherModule() + } +} + +// ----- + +firrtl.circuit "MutualLoop" { + firrtl.module @MutualLoop() { + firrtl.instance a @A() + } + firrtl.module @A() { + // expected-note @below {{A instantiates B here}} + firrtl.instance b @B() + } + // expected-error @below {{recursive instantiation}} + firrtl.module @B() { + // expected-note @below {{B instantiates A here}} + firrtl.instance a @A() + } +} + +// ----- + +// Should disallow recursive class instantiation. +firrtl.circuit "Classes" { + firrtl.module @Classes() { + firrtl.object @RecursiveClass() + } + + // expected-error @below {{recursive instantiation}} + firrtl.class @RecursiveClass() { + // expected-note @below {{RecursiveClass instantiates RecursiveClass here}} + %0 = firrtl.object @RecursiveClass() + } +} diff --git a/test/Dialect/FIRRTL/check-recursive-instantiation.mlir b/test/Dialect/FIRRTL/check-recursive-instantiation.mlir new file mode 100644 index 000000000000..c9ede2b247bb --- /dev/null +++ b/test/Dialect/FIRRTL/check-recursive-instantiation.mlir @@ -0,0 +1,6 @@ +// RUN: circt-opt -pass-pipeline='builtin.module(firrtl.circuit(firrtl-check-recursive-instantiation))' %s | FileCheck %s + +// CHECK-LABEL: firrtl.circuit "NoLoop" +firrtl.circuit "NoLoop" { + firrtl.module @NoLoop() { } +}