diff --git a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp index d32b5820ae..279bbce5de 100644 --- a/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp +++ b/lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp @@ -1711,27 +1711,42 @@ void GrandCentralPass::runOnOperation() { instancePaths = &instancePathCache; /// Contains the set of modules which are instantiated by the DUT, but not a - /// companion or instantiated by a companion. If no DUT exists, treat the top - /// module as if it were the DUT. This works by doing a depth-first walk of - /// the instance graph, starting from the "effective" DUT and stopping the - /// search at any modules which are known companions. - DenseSet dutModules; - FModuleOp effectiveDUT = dut; - if (!effectiveDUT) - effectiveDUT = cast( - *instancePaths->instanceGraph.getTopLevelNode()->getModule()); - auto dfRange = - llvm::depth_first(instancePaths->instanceGraph.lookup(effectiveDUT)); - for (auto i = dfRange.begin(), e = dfRange.end(); i != e;) { - auto module = cast(*i->getModule()); - if (AnnotationSet(module).hasAnnotation(companionAnnoClass)) { - i.skipChildren(); - continue; + /// companion, instantiated by a companion, or instantiated under a bind. If + /// no DUT exists, treat the top module as if it were the DUT. This works by + /// doing a depth-first walk of the instance graph, starting from the + /// "effective" DUT and stopping the search at any modules which are known + /// companions or any instances which are marked "lowerToBind". + DenseSet dutModules; + InstanceGraphNode *effectiveDUT; + if (dut) + effectiveDUT = instancePaths->instanceGraph.lookup(dut); + else + effectiveDUT = instancePaths->instanceGraph.getTopLevelNode(); + { + SmallVector modules({effectiveDUT}); + while (!modules.empty()) { + auto *m = modules.pop_back_val(); + for (InstanceRecord *a : *m) { + auto *mod = a->getTarget(); + // Skip modules that we've visited, that are are under the companion + // module, or are bound/under a layer block. + if (auto block = a->getInstance()->getParentOfType()) { + auto diag = a->getInstance().emitOpError() + << "is instantiated under a '" << block.getOperationName() + << "' op which is unexpected by GrandCentral (did you " + "forget to run the LowerLayers pass?)"; + diag.attachNote(block.getLoc()) + << "the '" << block.getOperationName() << "' op is here"; + removalError = true; + } + if (dutModules.contains(mod) || + AnnotationSet(mod->getModule()).hasAnnotation(companionAnnoClass) || + cast(*a->getInstance()).getLowerToBind()) + continue; + modules.push_back(mod); + dutModules.insert(mod); + } } - dutModules.insert(i->getModule()); - // Manually increment the iterator to avoid walking off the end from - // skipChildren. - ++i; } // Maybe return the lone instance of a module. Generate errors on the op if @@ -1765,7 +1780,6 @@ void GrandCentralPass::runOnOperation() { /// (2) the leafMap. Annotations are removed as they are discovered and if /// they are not malformed. DenseSet modulesToDelete; - removalError = false; circuitOp.walk([&](Operation *op) { TypeSwitch(op) .Case([&](auto op) { @@ -1950,12 +1964,11 @@ void GrandCentralPass::runOnOperation() { // Check to see if we should change the output directory of a // module. Only update in the following conditions: // 1) The module is the companion. - // 2) The module is NOT instantiated by the effective DUT. + // 2) The module is NOT instantiated by the effective DUT or + // is under a bind. auto *modNode = instancePaths->instanceGraph.lookup(mod); SmallVector instances(modNode->uses()); - if (modNode != companionNode && - dutModules.count( - modNode->getModule())) + if (modNode != companionNode && dutModules.count(modNode)) continue; LLVM_DEBUG({ diff --git a/test/Dialect/FIRRTL/grand-central-errors.mlir b/test/Dialect/FIRRTL/grand-central-errors.mlir index a09cbfa469..e921c7d8a2 100644 --- a/test/Dialect/FIRRTL/grand-central-errors.mlir +++ b/test/Dialect/FIRRTL/grand-central-errors.mlir @@ -375,3 +375,36 @@ firrtl.circuit "CompanionWithOutputs" firrtl.instance dut @DUT() } } + +// ----- + +firrtl.circuit "UnexpectedLayer" + attributes {annotations = [ + {class = "sifive.enterprise.grandcentral.AugmentedBundleType", + defName = "Foo", + elements = [ + {class = "sifive.enterprise.grandcentral.AugmentedBundleType", + defName = "Bar", + elements = [], + name = "bar"}], + id = 0 : i64, + name = "MyView"}]} { + firrtl.layer @A bind {} + firrtl.module private @MyView_companion() + attributes {annotations = [{ + class = "sifive.enterprise.grandcentral.ViewAnnotation.companion", + id = 0 : i64, + name = "MyView"}]} {} + firrtl.module private @Foo() {} + firrtl.module private @DUT() { + firrtl.instance MyView_companion @MyView_companion() + // expected-note @below {{the 'firrtl.layerblock' op is here}} + firrtl.layerblock @A { + // expected-error @below {{'firrtl.instance' op is instantiated under a 'firrtl.layerblock' op}} + firrtl.instance foo @Foo() + } + } + firrtl.module @UnexpectedLayer() { + firrtl.instance dut @DUT() + } +} diff --git a/test/firtool/lower-layers-directories.fir b/test/firtool/lower-layers-directories.fir new file mode 100644 index 0000000000..a50e823b01 --- /dev/null +++ b/test/firtool/lower-layers-directories.fir @@ -0,0 +1,132 @@ +; RUN: firtool %s | FileCheck %s --implicit-check-not FILE + +; This test checks output directory behavior of circuits with layers with other +; features. Generally, layer code is treated as "verification" code and should +; be kept out of the design area when `firtool` is run with options where the +; design area is explicitly specified. +; +; This test is expected to eventually be removed once the FIRRTL ABI switches to +; using filelists or manifests and a flat output directory structure. + +FIRRTL version 4.0.0 +circuit Testbench: %[[ + { + "class": "sifive.enterprise.grandcentral.ViewAnnotation", + "name": "MyView", + "companion": "~Testbench|GrandCentral", + "parent": "~Testbench|Testbench", + "view": { + "class": "sifive.enterprise.grandcentral.AugmentedBundleType", + "defName": "Interface", + "elements": [ + { + "name": "uint", + "description": "a wire called 'uint'", + "tpe": { + "class": "sifive.enterprise.grandcentral.AugmentedGroundType", + "ref": { + "circuit": "Testbench", + "module": "DUT", + "path": [], + "ref": "a", + "component": [] + }, + "tpe": { + "class": "sifive.enterprise.grandcentral.GrandCentralView$UnknownGroundType$" + } + } + } + ] + } + }, + { + "class": "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation", + "directory": "gct", + "filename": "gct/bindings.sv" + }, + { + "class":"firrtl.transforms.DontTouchAnnotation", + "target":"~Testbench|DUT>a" + }, + { + "class":"firrtl.transforms.DontTouchAnnotation", + "target":"~Testbench|Foo>a" + }, + { + "class":"firrtl.transforms.DontTouchAnnotation", + "target":"~Testbench|Bar>a" + }, + { + "class":"firrtl.transforms.DontTouchAnnotation", + "target":"~Testbench|Baz>a" + }, + { + "class":"firrtl.transforms.DontTouchAnnotation", + "target":"~Testbench|Qux>a" + }, + { + "class":"firrtl.transforms.DontTouchAnnotation", + "target":"~Testbench|Quz>a" + }, + { + "class": "sifive.enterprise.firrtl.MarkDUTAnnotation", + "target": "~Testbench|DUT" + }, + { + "class": "sifive.enterprise.firrtl.TestBenchDirAnnotation", + "dirname": "testbench" + } +]] + layer A, bind: + + module Foo: + node a = UInt<1>(0) + module Bar: + node a = UInt<1>(1) + module Baz: + node a = UInt<2>(2) + + module Qux: + node a = UInt<2>(3) + module Quz: + node a = UInt<3>(4) + + module GrandCentral: + inst qux of Qux + inst quz of Quz + + module DUT: + inst foo of Foo + inst baz of Baz + inst grandCentral of GrandCentral + + node a = UInt<3>(4) + + layerblock A: + inst bar of Bar + inst bazz of Baz + inst quz of Quz + + public module Testbench: + inst dut of DUT + +; The only modules NOT in a file are Foo, Baz, and DUT. +; +; CHECK: module Foo(); +; CHECK: module Baz(); +; CHECK: module DUT(); + +; The following modules are all in the Testbench directory. +; +; CHECK-DAG: FILE "testbench{{[/\]}}Bar.sv" +; CHECK-DAG: FILE "testbench{{[/\]}}DUT_A.sv" +; CHECK-DAG: FILE "testbench{{[/\]}}Testbench.sv" +; CHECK-DAG: FILE "testbench{{[/\]}}layers_Testbench_A.sv" + +; The following modules are all in the Grand Central directory. +; +; CHECK-DAG: FILE "gct{{[/\]}}GrandCentral.sv" +; CHECK-DAG: FILE "gct{{[/\]}}Interface.sv" +; CHECK-DAG: FILE "gct{{[/\]}}Qux.sv" +; CHECK-DAG: FILE "gct{{[/\]}}Quz.sv" +; CHECK-DAG: FILE "gct{{[/\]}}bindings.sv"