Skip to content

Commit

Permalink
[FIRRTL] Add CheckRecursiveInstantiation diagnostic pass
Browse files Browse the repository at this point in the history
This adds a pass to FIRRTL to check for recursive module instantiation,
which is illegal because it corresponds to infinitely sized hardware.
This uses the path based strongly connected component (SCC) algorithm
with some extra book keeping to track if there was a loop in the current
SCC or not.  This is especially helpful to detect if a single-element
SCC actually contains a loop or not.

This does not use the upstream SCCIterator because we hope that this
version of the algorithm is faster by a constant factor, we don't need
to visit the SCCs in post-order, and hopefully faster handling of cycles
in single-element SCCs (which requires re-scanning all edges from a node
using the upstream version).
  • Loading branch information
youngar committed Aug 7, 2024
1 parent 3189b61 commit 0a38617
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 0 deletions.
2 changes: 2 additions & 0 deletions include/circt/Dialect/FIRRTL/Passes.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ std::unique_ptr<mlir::Pass> createLowerDPIPass();
std::unique_ptr<mlir::Pass>
createAssignOutputDirsPass(mlir::StringRef outputDir = "");

std::unique_ptr<mlir::Pass> createCheckRecursiveInstantiation();

/// Generate the code for registering passes.
#define GEN_PASS_REGISTRATION
#include "circt/Dialect/FIRRTL/Passes.h.inc"
Expand Down
14 changes: 14 additions & 0 deletions include/circt/Dialect/FIRRTL/Passes.td
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions lib/Dialect/FIRRTL/Transforms/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ add_circt_dialect_library(CIRCTFIRRTLTransforms
AddSeqMemPorts.cpp
BlackBoxReader.cpp
CheckCombLoops.cpp
CheckRecursiveInstantiation.cpp
CreateCompanionAssume.cpp
CreateSiFiveMetadata.cpp
Dedup.cpp
Expand Down
96 changes: 96 additions & 0 deletions lib/Dialect/FIRRTL/Transforms/CheckRecursiveInstantiation.cpp
Original file line number Diff line number Diff line change
@@ -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<InstanceGraphNode *> 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<InstanceGraphNode *, 8> scc(nodes.begin(), nodes.end());

// A set of all nodes seen in the path.
llvm::SmallPtrSet<InstanceGraphNode *, 8> 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 &note = 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<InstanceGraph>();
if (failed(run(instanceGraph)))
return signalPassFailure();
markAllAnalysesPreserved();
} // namespace
};
} // namespace

std::unique_ptr<mlir::Pass> circt::firrtl::createCheckRecursiveInstantiation() {
return std::make_unique<CheckRecursiveInstantiationPass>();
}
70 changes: 70 additions & 0 deletions test/Dialect/FIRRTL/check-recursive-instantiation-errors.mlir
Original file line number Diff line number Diff line change
@@ -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()
}
}
6 changes: 6 additions & 0 deletions test/Dialect/FIRRTL/check-recursive-instantiation.mlir
Original file line number Diff line number Diff line change
@@ -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() { }
}

0 comments on commit 0a38617

Please sign in to comment.