Skip to content

Commit

Permalink
[FIRRTL] Fix bug in GCT Taps XMRs (#3415)
Browse files Browse the repository at this point in the history 
Fix a bug in Grand Central (GCT) Data/Mem Taps pass where Data Taps
could be produced with invalid XMRs.  An exhaustive test case is added
that checks all permutations of upwards and downwards XMRs including
corner cases where the tap module is a leaf and the root.

Fixes #3414.
  • Loading branch information
@seldridge
seldridge authored Jun 24, 2022
1 parent a84462d commit 69b3f68
Show file tree
Hide file tree
Showing 2 changed files with 206 additions and 28 deletions.
43 changes: 15 additions & 28 deletions lib/Dialect/FIRRTL/Transforms/GrandCentralTaps.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -830,9 +830,14 @@ void GrandCentralTapsPass::runOnOperation() {
<< " - Shortest prefix " << *shortestPrefix << "\n");

// Determine the module at which the hierarchical name should start.
Operation *opInRootModule =
shortestPrefix->empty() ? path.front() : shortestPrefix->front();
auto rootModule = opInRootModule->getParentOfType<FModuleLike>();
FModuleLike rootModule;
if (shortestPrefix->empty()) {
if (port.target.hasPort())
rootModule = cast<FModuleLike>(port.target.getOp());
else
rootModule = port.target.getOp()->getParentOfType<FModuleLike>();
} else
rootModule = shortestPrefix->front()->getParentOfType<FModuleLike>();

SmallVector<Attribute> symbols;
SmallString<128> hname;
Expand All @@ -847,36 +852,18 @@ void GrandCentralTapsPass::runOnOperation() {
// Concatenate the prefix into a proper full hierarchical name.
addSymbol(
FlatSymbolRefAttr::get(SymbolTable::getSymbolName(rootModule)));
if (port.nla && shortestPrefix->empty() &&
port.nla.root() != rootModule.moduleNameAttr()) {
// This handles the case when nla is not considered for common prefix
// stripping.
auto rootMod = port.nla.root();
for (auto p : path) {
if (rootMod == p->getParentOfType<FModuleOp>().getNameAttr())
break;
addSymbol(getInnerRefTo(p));
}
}
for (auto inst : shortestPrefix.getValue())
addSymbol(getInnerRefTo(inst));
if (port.nla) {
auto namepath = port.nla.namepath();
Attribute leaf = namepath.getValue().back();
if (!leaf.isa<InnerRefAttr>()) {
if (port.target.hasPort())
leaf = getInnerRefTo(port.target.getOp(), port.target.getPort());
else
leaf = getInnerRefTo(port.target.getOp());
}
addSymbol(leaf);
} else if (port.target.getOp()) {

if (port.target.getOp()) {
Attribute leaf;
if (port.target.hasPort())
addSymbol(
getInnerRefTo(port.target.getOp(), port.target.getPort()));
leaf = getInnerRefTo(port.target.getOp(), port.target.getPort());
else
addSymbol(getInnerRefTo(port.target.getOp()));
leaf = getInnerRefTo(port.target.getOp());
addSymbol(leaf);
}

if (!port.suffix.empty()) {
hname += '.';
hname += port.suffix;
Expand Down
191 changes: 191 additions & 0 deletions test/Dialect/FIRRTL/SFCTests/data-taps-nlas.fir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,194 @@ circuit TestHarness : %[[
; CHECK: module DataTap
; CHECK-NOT: endmodule
; CHECK: assign _0 = Top.test.signal;

; // -----

circuit Top : %[[
{
"class":"sifive.enterprise.grandcentral.DataTapsAnnotation",
"blackBox":"~Top|DataTap_Submodule",
"keys":[
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT/submodule:Submodule>wire_Submodule",
"portName":"~Top|DataTap_Submodule>_0"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT>wire_DUT",
"portName":"~Top|DataTap_Submodule>_1"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top>wire_Top",
"portName":"~Top|DataTap_Submodule>_2"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT/submodule:Submodule>port_Submodule",
"portName":"~Top|DataTap_Submodule>_3"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT>port_DUT",
"portName":"~Top|DataTap_Submodule>_4"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top>port_Top",
"portName":"~Top|DataTap_Submodule>_5"
}
]
},
{
"class":"sifive.enterprise.grandcentral.DataTapsAnnotation",
"blackBox":"~Top|DataTap_DUT",
"keys":[
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT/submodule:Submodule>wire_Submodule",
"portName":"~Top|DataTap_DUT>_0"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT>wire_DUT",
"portName":"~Top|DataTap_DUT>_1"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top>wire_Top",
"portName":"~Top|DataTap_DUT>_2"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT/submodule:Submodule>port_Submodule",
"portName":"~Top|DataTap_DUT>_3"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT>port_DUT",
"portName":"~Top|DataTap_DUT>_4"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top>port_Top",
"portName":"~Top|DataTap_DUT>_5"
}
]
},
{
"class":"sifive.enterprise.grandcentral.DataTapsAnnotation",
"blackBox":"~Top|DataTap_Top",
"keys":[
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT/submodule:Submodule>wire_Submodule",
"portName":"~Top|DataTap_Top>_0"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT>wire_DUT",
"portName":"~Top|DataTap_Top>_1"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top>wire_Top",
"portName":"~Top|DataTap_Top>_2"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT/submodule:Submodule>port_Submodule",
"portName":"~Top|DataTap_Top>_3"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top/dut:DUT>port_DUT",
"portName":"~Top|DataTap_Top>_4"
},
{
"class":"sifive.enterprise.grandcentral.ReferenceDataTapKey",
"source":"~Top|Top>port_Top",
"portName":"~Top|DataTap_Top>_5"
}
]
}
]]
extmodule DataTap_Submodule :
output _0 : UInt<1>
output _1 : UInt<1>
output _2 : UInt<1>
output _3 : UInt<1>
output _4 : UInt<1>
output _5 : UInt<1>


extmodule DataTap_DUT :
output _0 : UInt<1>
output _1 : UInt<1>
output _2 : UInt<1>
output _3 : UInt<1>
output _4 : UInt<1>
output _5 : UInt<1>

extmodule DataTap_Top :
output _0 : UInt<1>
output _1 : UInt<1>
output _2 : UInt<1>
output _3 : UInt<1>
output _4 : UInt<1>
output _5 : UInt<1>

module Submodule :
output port_Submodule: UInt<1>
port_Submodule is invalid

wire wire_Submodule: UInt<1>
wire_Submodule is invalid

inst tap of DataTap_Submodule

module DUT :
output port_DUT: UInt<1>
port_DUT is invalid

wire wire_DUT: UInt<1>
wire_DUT is invalid

inst submodule of Submodule

inst tap of DataTap_DUT

module Top :
output port_Top : UInt<1>
port_Top is invalid

wire wire_Top: UInt<1>
wire_Top is invalid

inst dut of DUT
inst tap of DataTap_Top

; CHECK: module DataTap_Submodule
; CHECK: assign _0 = Submodule.wire_Submodule
; CHECK-NEXT: assign _1 = DUT.wire_DUT
; CHECK-NEXT: assign _2 = Top.wire_Top
; CHECK: assign _3 = Submodule.port_Submodule
; CHECK-NEXT: assign _4 = DUT.port_DUT
; CHECK-NEXT: assign _5 = Top.port_Top

; CHECK: module DataTap_DUT
; CHECK: assign _0 = DUT.submodule.wire_Submodule
; CHECK-NEXT: assign _1 = DUT.wire_DUT
; CHECK-NEXT: assign _2 = Top.wire_Top
; CHECK: assign _3 = DUT.submodule.port_Submodule
; CHECK-NEXT: assign _4 = DUT.port_DUT
; CHECK-NEXT: assign _5 = Top.port_Top

; CHECK: module DataTap_Top
; CHECK: assign _0 = Top.dut.submodule.wire_Submodule
; CHECK-NEXT: assign _1 = Top.dut.wire_DUT
; CHECK-NEXT: assign _2 = Top.wire_Top
; CHECK: assign _3 = Top.dut.submodule.port_Submodule
; CHECK-NEXT: assign _4 = Top.dut.port_DUT
; CHECK-NEXT: assign _5 = Top.port_Top

0 comments on commit 69b3f68

Please sign in to comment.