From f85865eea9471a74b5353bbf68532d76bf627198 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Mon, 21 Oct 2024 12:47:10 -0400 Subject: [PATCH] [FIRRTL] Convert CheckLayers to use InstanceInfo (#7635) Change the `CheckLayers` pass to use the `InstanceInfo` analysis. This new analysis makes the error checking trivial. However, the error reporting is still somewhat tedious. Signed-off-by: Schuyler Eldridge --- lib/Dialect/FIRRTL/Transforms/CheckLayers.cpp | 113 ++++++++---------- 1 file changed, 53 insertions(+), 60 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/CheckLayers.cpp b/lib/Dialect/FIRRTL/Transforms/CheckLayers.cpp index 5f28d7cdb236..5d0d7f1b209a 100644 --- a/lib/Dialect/FIRRTL/Transforms/CheckLayers.cpp +++ b/lib/Dialect/FIRRTL/Transforms/CheckLayers.cpp @@ -6,10 +6,13 @@ // //===----------------------------------------------------------------------===// +#include "circt/Analysis/FIRRTLInstanceInfo.h" #include "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" #include "circt/Dialect/FIRRTL/FIRRTLOps.h" #include "circt/Dialect/FIRRTL/Passes.h" +#include "circt/Support/InstanceGraphInterface.h" #include "mlir/Pass/Pass.h" +#include "llvm/ADT/DepthFirstIterator.h" #include "llvm/ADT/PostOrderIterator.h" namespace circt { @@ -25,72 +28,56 @@ using namespace mlir; namespace { class CheckLayers { - CheckLayers(InstanceGraph &instanceGraph) : instanceGraph(instanceGraph) {} - - /// Walk the LayerBlock and report any illegal instantiation found within. - void run(LayerBlockOp layerBlock) { - layerBlock.getBody()->walk([&](FInstanceLike instance) { - auto moduleName = instance.getReferencedModuleNameAttr(); - auto *targetModule = - instanceGraph.lookup(moduleName)->getModule(); - auto childLayerBlock = layerBlocks.lookup(targetModule); - if (childLayerBlock) { - auto diag = emitError(instance.getLoc()) - << "cannot instantiate " << moduleName - << " under a layerblock, because " << moduleName - << " contains a layerblock"; - diag.attachNote(childLayerBlock.getLoc()) << "layerblock here"; - error = true; - } - }); - } + CheckLayers(InstanceGraph &instanceGraph, InstanceInfo &instanceInfo) + : iGraph(instanceGraph), iInfo(instanceInfo) {} /// Walk a module, reporting any illegal instantation of layers under layers, /// and record if this module contains any layerblocks. void run(FModuleOp moduleOp) { - // If this module directly contains a layerblock, or instantiates another - // module with a layerblock, then this will point to the layerblock. We use - // this for error reporting. We keep track of only the first layerblock - // found. - LayerBlockOp firstLayerblock = nullptr; - moduleOp.getBodyBlock()->walk([&](Operation *op) { - // If we are instantiating a module, check if it contains a layerblock. If - // it does, mark this target layerblock as our layerblock. - if (auto instance = dyn_cast(op)) { - // If this is the first layer block in this module, record it. - if (!firstLayerblock) { - auto moduleName = instance.getReferencedModuleNameAttr(); - auto *targetModule = - instanceGraph.lookup(moduleName)->getModule().getOperation(); - firstLayerblock = layerBlocks.lookup(targetModule); + // No instance is under a layer block. No further examination is necessary. + if (!iInfo.anyInstanceUnderLayer(moduleOp)) + return; + + // The module is under a layer block. Verify that it has no layer blocks. + LayerBlockOp layerBlockOp; + moduleOp.getBodyBlock()->walk([&](LayerBlockOp op) { + layerBlockOp = op; + return WalkResult::interrupt(); + }); + if (!layerBlockOp) + return; + + // The module contains layer blocks and is instantiated under a layer block. + // Walk up the instance hierarchy to find the first instance which is + // directly under a layer block. + error = true; + for (auto *node : llvm::inverse_depth_first(iGraph.lookup(moduleOp))) { + auto modOp = node->getModule(); + if (previousErrors.contains(node) || !iInfo.anyInstanceUnderLayer(modOp)) + continue; + for (auto *instNode : node->uses()) { + auto instanceOp = instNode->getInstance(); + if (instanceOp->getParentOfType()) { + auto moduleName = modOp.getModuleNameAttr(); + auto diag = emitError(instanceOp.getLoc()) + << "cannot instantiate " << moduleName + << " under a layerblock, because " << moduleName + << " contains a layerblock"; + diag.attachNote(layerBlockOp->getLoc()) << "layerblock here"; + previousErrors.insert(node); + continue; } - return WalkResult::advance(); - } - if (auto layerBlock = dyn_cast(op)) { - // If this is the first layer block in this module, record it. - if (!firstLayerblock) - firstLayerblock = layerBlock; - // Process the layerblock. - run(layerBlock); - // Don't recurse on elements of the layerblock. If an instance within - // did contain a layerblock, then an error would have been reported for - // it already. - return WalkResult::skip(); } - // Do nothing for all other operations. - return WalkResult::advance(); - }); - // If this module contained a layerblock, then record it. - if (firstLayerblock) - layerBlocks.try_emplace(moduleOp, firstLayerblock); + } } public: - static LogicalResult run(InstanceGraph &instanceGraph) { - CheckLayers checkLayers(instanceGraph); + static LogicalResult run(InstanceGraph &instanceGraph, + InstanceInfo &instanceInfo) { + CheckLayers checkLayers(instanceGraph, instanceInfo); DenseSet visited; for (auto *root : instanceGraph) { - for (auto *node : llvm::post_order_ext(root, visited)) { + for (auto *node : llvm::inverse_post_order_ext(root, visited)) { if (auto moduleOp = dyn_cast(node->getModule())) checkLayers.run(moduleOp); } @@ -99,10 +86,15 @@ class CheckLayers { } private: - InstanceGraph &instanceGraph; - /// A mapping from a module to the first layerblock that it contains, - /// transitively through instances. - DenseMap layerBlocks; + /// Pre-populated analyses + InstanceGraph &iGraph; + InstanceInfo &iInfo; + + /// This records modules for which we have already generated errors when doing + /// a top-down walk. + DenseSet previousErrors; + + /// Indicates if this checker found an error. bool error = false; }; } // end anonymous namespace @@ -111,7 +103,8 @@ class CheckLayersPass : public circt::firrtl::impl::CheckLayersBase { public: void runOnOperation() override { - if (failed(CheckLayers::run(getAnalysis()))) + if (failed(CheckLayers::run(getAnalysis(), + getAnalysis()))) return signalPassFailure(); markAllAnalysesPreserved(); }