Skip to content

Commit 679737a

Browse files
seldridgeseldridge
committed
[FIRRTL] Fix Grand Central and Layers Directories
Fix a bug involving interactions of Grand Central and layers output directories. When compiling a design that has a DUT and a testbench directory, treat modules that are instantiated by Grand Central and under a bound module (which is lowered from a layer) as belonging to Grand Central and should be placed in its output directory. This will be revisited in the future as the FIRRTL ABI moves more towards filelists or manifests. Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
1 parent cafb5e9 commit 679737a

File tree

3 files changed

+203
-25
lines changed

3 files changed

+203
-25
lines changed

‎lib/Dialect/FIRRTL/Transforms/GrandCentral.cpp

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1711,27 +1711,42 @@ void GrandCentralPass::runOnOperation() {
17111711
instancePaths = &instancePathCache;
17121712

17131713
/// Contains the set of modules which are instantiated by the DUT, but not a
1714-
/// companion or instantiated by a companion. If no DUT exists, treat the top
1715-
/// module as if it were the DUT. This works by doing a depth-first walk of
1716-
/// the instance graph, starting from the "effective" DUT and stopping the
1717-
/// search at any modules which are known companions.
1718-
DenseSet<igraph::ModuleOpInterface> dutModules;
1719-
FModuleOp effectiveDUT = dut;
1720-
if (!effectiveDUT)
1721-
effectiveDUT = cast<FModuleOp>(
1722-
*instancePaths->instanceGraph.getTopLevelNode()->getModule());
1723-
auto dfRange =
1724-
llvm::depth_first(instancePaths->instanceGraph.lookup(effectiveDUT));
1725-
for (auto i = dfRange.begin(), e = dfRange.end(); i != e;) {
1726-
auto module = cast<FModuleLike>(*i->getModule());
1727-
if (AnnotationSet(module).hasAnnotation(companionAnnoClass)) {
1728-
i.skipChildren();
1729-
continue;
1714+
/// companion, instantiated by a companion, or instantiated under a bind. If
1715+
/// no DUT exists, treat the top module as if it were the DUT. This works by
1716+
/// doing a depth-first walk of the instance graph, starting from the
1717+
/// "effective" DUT and stopping the search at any modules which are known
1718+
/// companions or any instances which are marked "lowerToBind".
1719+
DenseSet<InstanceGraphNode *> dutModules;
1720+
InstanceGraphNode *effectiveDUT;
1721+
if (dut)
1722+
effectiveDUT = instancePaths->instanceGraph.lookup(dut);
1723+
else
1724+
effectiveDUT = instancePaths->instanceGraph.getTopLevelNode();
1725+
{
1726+
SmallVector<InstanceGraphNode *> modules({effectiveDUT});
1727+
while (!modules.empty()) {
1728+
auto *m = modules.pop_back_val();
1729+
for (InstanceRecord *a : *m) {
1730+
auto *mod = a->getTarget();
1731+
// Skip modules that we've visited, that are are under the companion
1732+
// module, or are bound/under a layer block.
1733+
if (auto block = a->getInstance()->getParentOfType<LayerBlockOp>()) {
1734+
auto diag = a->getInstance().emitOpError()
1735+
<< "is instantiated under a '" << block.getOperationName()
1736+
<< "' op which is unexpected by GrandCentral (did you "
1737+
"forget to run the LowerLayers pass?)";
1738+
diag.attachNote(block.getLoc())
1739+
<< "the '" << block.getOperationName() << "' op is here";
1740+
removalError = true;
1741+
}
1742+
if (dutModules.contains(mod) ||
1743+
AnnotationSet(mod->getModule()).hasAnnotation(companionAnnoClass) ||
1744+
cast<InstanceOp>(*a->getInstance()).getLowerToBind())
1745+
continue;
1746+
modules.push_back(mod);
1747+
dutModules.insert(mod);
1748+
}
17301749
}
1731-
dutModules.insert(i->getModule<igraph::ModuleOpInterface>());
1732-
// Manually increment the iterator to avoid walking off the end from
1733-
// skipChildren.
1734-
++i;
17351750
}
17361751

17371752
// Maybe return the lone instance of a module. Generate errors on the op if
@@ -1765,7 +1780,6 @@ void GrandCentralPass::runOnOperation() {
17651780
/// (2) the leafMap. Annotations are removed as they are discovered and if
17661781
/// they are not malformed.
17671782
DenseSet<Operation *> modulesToDelete;
1768-
removalError = false;
17691783
circuitOp.walk([&](Operation *op) {
17701784
TypeSwitch<Operation *>(op)
17711785
.Case<RegOp, RegResetOp, WireOp, NodeOp>([&](auto op) {
@@ -1950,12 +1964,11 @@ void GrandCentralPass::runOnOperation() {
19501964
// Check to see if we should change the output directory of a
19511965
// module. Only update in the following conditions:
19521966
// 1) The module is the companion.
1953-
// 2) The module is NOT instantiated by the effective DUT.
1967+
// 2) The module is NOT instantiated by the effective DUT or
1968+
// is under a bind.
19541969
auto *modNode = instancePaths->instanceGraph.lookup(mod);
19551970
SmallVector<InstanceRecord *> instances(modNode->uses());
1956-
if (modNode != companionNode &&
1957-
dutModules.count(
1958-
modNode->getModule<igraph::ModuleOpInterface>()))
1971+
if (modNode != companionNode && dutModules.count(modNode))
19591972
continue;
19601973

19611974
LLVM_DEBUG({

‎test/Dialect/FIRRTL/grand-central-errors.mlir

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,3 +375,36 @@ firrtl.circuit "CompanionWithOutputs"
375375
firrtl.instance dut @DUT()
376376
}
377377
}
378+
379+
// -----
380+
381+
firrtl.circuit "UnexpectedLayer"
382+
attributes {annotations = [
383+
{class = "sifive.enterprise.grandcentral.AugmentedBundleType",
384+
defName = "Foo",
385+
elements = [
386+
{class = "sifive.enterprise.grandcentral.AugmentedBundleType",
387+
defName = "Bar",
388+
elements = [],
389+
name = "bar"}],
390+
id = 0 : i64,
391+
name = "MyView"}]} {
392+
firrtl.layer @A bind {}
393+
firrtl.module private @MyView_companion()
394+
attributes {annotations = [{
395+
class = "sifive.enterprise.grandcentral.ViewAnnotation.companion",
396+
id = 0 : i64,
397+
name = "MyView"}]} {}
398+
firrtl.module private @Foo() {}
399+
firrtl.module private @DUT() {
400+
firrtl.instance MyView_companion @MyView_companion()
401+
// expected-note @below {{the 'firrtl.layerblock' op is here}}
402+
firrtl.layerblock @A {
403+
// expected-error @below {{'firrtl.instance' op is instantiated under a 'firrtl.layerblock' op}}
404+
firrtl.instance foo @Foo()
405+
}
406+
}
407+
firrtl.module @UnexpectedLayer() {
408+
firrtl.instance dut @DUT()
409+
}
410+
}

‎test/firtool/lower-layers-directories.fir

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
; RUN: firtool %s | FileCheck %s --implicit-check-not FILE
2+
3+
; This test checks output directory behavior of circuits with layers with other
4+
; features. Generally, layer code is treated as "verification" code and should
5+
; be kept out of the design area when `firtool` is run with options where the
6+
; design area is explicitly specified.
7+
;
8+
; This test is expected to eventually be removed once the FIRRTL ABI switches to
9+
; using filelists or manifests and a flat output directory structure.
10+
11+
FIRRTL version 4.0.0
12+
circuit Testbench: %[[
13+
{
14+
"class": "sifive.enterprise.grandcentral.ViewAnnotation",
15+
"name": "MyView",
16+
"companion": "~Testbench|GrandCentral",
17+
"parent": "~Testbench|Testbench",
18+
"view": {
19+
"class": "sifive.enterprise.grandcentral.AugmentedBundleType",
20+
"defName": "Interface",
21+
"elements": [
22+
{
23+
"name": "uint",
24+
"description": "a wire called 'uint'",
25+
"tpe": {
26+
"class": "sifive.enterprise.grandcentral.AugmentedGroundType",
27+
"ref": {
28+
"circuit": "Testbench",
29+
"module": "DUT",
30+
"path": [],
31+
"ref": "a",
32+
"component": []
33+
},
34+
"tpe": {
35+
"class": "sifive.enterprise.grandcentral.GrandCentralView$UnknownGroundType$"
36+
}
37+
}
38+
}
39+
]
40+
}
41+
},
42+
{
43+
"class": "sifive.enterprise.grandcentral.ExtractGrandCentralAnnotation",
44+
"directory": "gct",
45+
"filename": "gct/bindings.sv"
46+
},
47+
{
48+
"class":"firrtl.transforms.DontTouchAnnotation",
49+
"target":"~Testbench|DUT>a"
50+
},
51+
{
52+
"class":"firrtl.transforms.DontTouchAnnotation",
53+
"target":"~Testbench|Foo>a"
54+
},
55+
{
56+
"class":"firrtl.transforms.DontTouchAnnotation",
57+
"target":"~Testbench|Bar>a"
58+
},
59+
{
60+
"class":"firrtl.transforms.DontTouchAnnotation",
61+
"target":"~Testbench|Baz>a"
62+
},
63+
{
64+
"class":"firrtl.transforms.DontTouchAnnotation",
65+
"target":"~Testbench|Qux>a"
66+
},
67+
{
68+
"class":"firrtl.transforms.DontTouchAnnotation",
69+
"target":"~Testbench|Quz>a"
70+
},
71+
{
72+
"class": "sifive.enterprise.firrtl.MarkDUTAnnotation",
73+
"target": "~Testbench|DUT"
74+
},
75+
{
76+
"class": "sifive.enterprise.firrtl.TestBenchDirAnnotation",
77+
"dirname": "testbench"
78+
}
79+
]]
80+
layer A, bind:
81+
82+
module Foo:
83+
node a = UInt<1>(0)
84+
module Bar:
85+
node a = UInt<1>(1)
86+
module Baz:
87+
node a = UInt<2>(2)
88+
89+
module Qux:
90+
node a = UInt<2>(3)
91+
module Quz:
92+
node a = UInt<3>(4)
93+
94+
module GrandCentral:
95+
inst qux of Qux
96+
inst quz of Quz
97+
98+
module DUT:
99+
inst foo of Foo
100+
inst baz of Baz
101+
inst grandCentral of GrandCentral
102+
103+
node a = UInt<3>(4)
104+
105+
layerblock A:
106+
inst bar of Bar
107+
inst bazz of Baz
108+
inst quz of Quz
109+
110+
public module Testbench:
111+
inst dut of DUT
112+
113+
; The only modules NOT in a file are Foo, Baz, and DUT.
114+
;
115+
; CHECK: module Foo();
116+
; CHECK: module Baz();
117+
; CHECK: module DUT();
118+
119+
; The following modules are all in the Testbench directory.
120+
;
121+
; CHECK-DAG: FILE "testbench{{[/\]}}Bar.sv"
122+
; CHECK-DAG: FILE "testbench{{[/\]}}DUT_A.sv"
123+
; CHECK-DAG: FILE "testbench{{[/\]}}Testbench.sv"
124+
; CHECK-DAG: FILE "testbench{{[/\]}}layers_Testbench_A.sv"
125+
126+
; The following modules are all in the Grand Central directory.
127+
;
128+
; CHECK-DAG: FILE "gct{{[/\]}}GrandCentral.sv"
129+
; CHECK-DAG: FILE "gct{{[/\]}}Interface.sv"
130+
; CHECK-DAG: FILE "gct{{[/\]}}Qux.sv"
131+
; CHECK-DAG: FILE "gct{{[/\]}}Quz.sv"
132+
; CHECK-DAG: FILE "gct{{[/\]}}bindings.sv"

0 commit comments

Comments
 (0)