Skip to content

Commit 77553d7

Browse files
prithayanprithayan
authored
[ExtractInstances] Append original instance name to path in metadata (#7872)
Update the metadata generated by ExtractInstances. Currently the path in the metadata, doesn't contain the original instance name, which causes difficulty in distinguishing extracted instances within the same module. This PR updates the metadata to append the original instance name to the path. Note that this instance name is of the instance, that has been extracted, hence it cannot be represented by the Hierarchical path.
1 parent 1e17dcf commit 77553d7

File tree

3 files changed

+57
-37
lines changed

3 files changed

+57
-37
lines changed

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

+33-17
Original file line numberDiff line numberDiff line change
@@ -150,19 +150,22 @@ struct ExtractInstancesPass
150150
/// hierarchy, but before being grouped into an optional submodule.
151151
SmallVector<std::pair<InstanceOp, ExtractionInfo>> extractedInstances;
152152

153-
// The uniquified wiring prefix for each instance.
154-
DenseMap<Operation *, SmallString<16>> instPrefices;
153+
// The uniquified wiring prefix and original name for each instance.
154+
DenseMap<Operation *, std::pair<SmallString<16>, StringAttr>>
155+
instPrefixNamesPair;
155156

156157
/// The current circuit namespace valid within the call to `runOnOperation`.
157158
CircuitNamespace circuitNamespace;
158159
/// Cached module namespaces.
159160
DenseMap<Operation *, hw::InnerSymbolNamespace> moduleNamespaces;
160161
/// The metadata class ops.
161162
ClassOp extractMetadataClass, schemaClass;
162-
const unsigned prefixNameFieldId = 0, pathFieldId = 2, fileNameFieldId = 4;
163+
const unsigned prefixNameFieldId = 0, pathFieldId = 2, fileNameFieldId = 4,
164+
instNameFieldId = 6;
163165
/// Cache of the inner ref to the new instances created. Will be used to
164166
/// create a path to the instance
165167
DenseMap<InnerRefAttr, InstanceOp> innerRefToInstances;
168+
Type stringType, pathType;
166169
};
167170
} // end anonymous namespace
168171

@@ -177,13 +180,16 @@ void ExtractInstancesPass::runOnOperation() {
177180
extractionPaths.clear();
178181
originalInstanceParents.clear();
179182
extractedInstances.clear();
180-
instPrefices.clear();
183+
instPrefixNamesPair.clear();
181184
moduleNamespaces.clear();
182185
circuitNamespace.clear();
183186
circuitNamespace.add(circuitOp);
184187
innerRefToInstances.clear();
185188
extractMetadataClass = {};
186189
schemaClass = {};
190+
auto *context = circuitOp->getContext();
191+
stringType = StringType::get(context);
192+
pathType = PathType::get(context);
187193

188194
// Walk the IR and gather all the annotations relevant for extraction that
189195
// appear on instances and the instantiated modules.
@@ -495,8 +501,10 @@ void ExtractInstancesPass::extractInstances() {
495501
// of the pass does, which would group instances to be extracted by prefix
496502
// and then iterate over them with the index in the group being used as `N`.
497503
StringRef prefix;
504+
auto &instPrefixEntry = instPrefixNamesPair[inst];
505+
instPrefixEntry.second = inst.getInstanceNameAttr();
498506
if (!info.prefix.empty()) {
499-
auto &prefixSlot = instPrefices[inst];
507+
auto &prefixSlot = instPrefixEntry.first;
500508
if (prefixSlot.empty()) {
501509
auto idx = prefixUniqueIDs[info.prefix]++;
502510
(Twine(info.prefix) + "_" + Twine(idx)).toVector(prefixSlot);
@@ -635,13 +643,13 @@ void ExtractInstancesPass::extractInstances() {
635643
// new instance we create inherit the wiring prefix, and all additional
636644
// new instances (e.g. through multiple instantiation of the parent) will
637645
// pick a new prefix.
638-
auto oldPrefix = instPrefices.find(inst);
639-
if (oldPrefix != instPrefices.end()) {
640-
LLVM_DEBUG(llvm::dbgs()
641-
<< " - Reusing prefix `" << oldPrefix->second << "`\n");
646+
auto oldPrefix = instPrefixNamesPair.find(inst);
647+
if (oldPrefix != instPrefixNamesPair.end()) {
648+
LLVM_DEBUG(llvm::dbgs() << " - Reusing prefix `"
649+
<< oldPrefix->second.first << "`\n");
642650
auto newPrefix = std::move(oldPrefix->second);
643-
instPrefices.erase(oldPrefix);
644-
instPrefices.insert({newInst, newPrefix});
651+
instPrefixNamesPair.erase(oldPrefix);
652+
instPrefixNamesPair.insert({newInst, newPrefix});
645653
}
646654

647655
// Inherit the old instance's extraction path.
@@ -885,7 +893,7 @@ void ExtractInstancesPass::groupInstances() {
885893
ports.clear();
886894
for (auto inst : insts) {
887895
// Determine the ports for the wrapper.
888-
StringRef prefix(instPrefices[inst]);
896+
StringRef prefix(instPrefixNamesPair[inst].first);
889897
unsigned portNum = inst.getNumResults();
890898
for (unsigned portIdx = 0; portIdx < portNum; ++portIdx) {
891899
auto name = inst.getPortNameStr(portIdx);
@@ -1049,7 +1057,8 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) {
10491057
auto file = getOrCreateFile(fileName);
10501058
auto builder = OpBuilder::atBlockEnd(file.getBody());
10511059
for (auto inst : insts) {
1052-
StringRef prefix(instPrefices[inst]);
1060+
StringRef prefix(instPrefixNamesPair[inst].first);
1061+
StringAttr origInstName(instPrefixNamesPair[inst].second);
10531062
if (prefix.empty()) {
10541063
LLVM_DEBUG(llvm::dbgs() << " - Skipping `" << inst.getName()
10551064
<< "` since it has no extraction prefix\n");
@@ -1089,6 +1098,11 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) {
10891098
builderOM.create<StringConstantOp>(builder.getStringAttr(fileName));
10901099
builderOM.create<PropAssignOp>(fFile, fileNameOp);
10911100

1101+
auto finstName =
1102+
builderOM.create<ObjectSubfieldOp>(object, instNameFieldId);
1103+
auto instNameOp = builderOM.create<StringConstantOp>(origInstName);
1104+
builderOM.create<PropAssignOp>(finstName, instNameOp);
1105+
10921106
// Now add this to the output field of the class.
10931107
classFields.emplace_back(object, prefix + "_field");
10941108
}
@@ -1112,6 +1126,7 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) {
11121126
os << ".";
11131127
addSymbol(sym);
11141128
}
1129+
os << "." << origInstName.getValue();
11151130
// The final instance name is excluded as this does not provide useful
11161131
// additional information and could conflict with a name inside the final
11171132
// module.
@@ -1157,11 +1172,12 @@ void ExtractInstancesPass::createSchema() {
11571172
auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd(
11581173
unknownLoc, circuitOp.getBodyBlock());
11591174
mlir::Type portsType[] = {
1160-
StringType::get(context), // name
1161-
PathType::get(context), // extracted instance path
1162-
StringType::get(context) // filename
1175+
stringType, // name
1176+
pathType, // extracted instance path
1177+
stringType, // filename
1178+
stringType // instance name
11631179
};
1164-
StringRef portFields[] = {"name", "path", "filename"};
1180+
StringRef portFields[] = {"name", "path", "filename", "inst_name"};
11651181

11661182
schemaClass = builderOM.create<ClassOp>("ExtractInstancesSchema", portFields,
11671183
portsType);

‎test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ firrtl.circuit "ExtractClockGatesMultigrouping" attributes {annotations = [{clas
4747
}
4848
// CHECK: emit.file "ClockGates.txt" {
4949
// CHECK-NEXT: sv.verbatim
50-
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.{{1}}.{{2}}\0A
51-
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.{{3}}\0A
50+
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.{{1}}.{{2}}.gate\0A
51+
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.{{3}}.gate\0A
5252
// CHECK-SAME: symbols = [
5353
// CHECK-SAME: @DUTModule
5454
// CHECK-SAME: #hw.innerNameRef<@DUTModule::[[INJMOD_SYM]]>

‎test/Dialect/FIRRTL/extract-instances.mlir

+22-18
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi
6464
// CHECK: firrtl.propassign %[[extractedInstances_field_0]], %extract_instances_metadata : !firrtl.class<@ExtractInstancesMetadata
6565
// CHECK: }
6666

67-
// CHECK: firrtl.class @ExtractInstancesSchema(in %name_in: !firrtl.string, out %name: !firrtl.string, in %path_in: !firrtl.path, out %path: !firrtl.path, in %filename_in: !firrtl.string, out %filename: !firrtl.string) {
67+
// CHECK: firrtl.class @ExtractInstancesSchema(in %name_in: !firrtl.string, out %name: !firrtl.string, in %path_in: !firrtl.path, out %path: !firrtl.path, in %filename_in: !firrtl.string, out %filename: !firrtl.string, in %inst_name_in: !firrtl.string, out %inst_name: !firrtl.string) {
6868
// CHECK: firrtl.propassign %name, %name_in : !firrtl.string
6969
// CHECK: firrtl.propassign %path, %path_in : !firrtl.path
7070
// CHECK: firrtl.propassign %filename, %filename_in : !firrtl.string
71+
// CHECK: firrtl.propassign %inst_name, %inst_name_in : !firrtl.string
7172
// CHECK: }
7273

7374
// CHECK: firrtl.class @ExtractInstancesMetadata(out %[[bb_0_field]]: !firrtl.class<@ExtractInstancesSchema
@@ -82,12 +83,15 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi
8283
// CHECK: %[[V4:.+]] = firrtl.object.subfield %[[bb_0]][filename_in]
8384
// CHECK: %[[V5:.+]] = firrtl.string "BlackBoxes.txt"
8485
// CHECK: firrtl.propassign %[[V4]], %[[V5]] : !firrtl.string
86+
// CHECK: %[[V6:.+]] = firrtl.object.subfield %bb_0[inst_name_in]
87+
// CHECK: %[[V7:.+]] = firrtl.string "bb"
88+
// CHECK; firrtl.propassign %[[V6]], %[[V7]] : !firrtl.string
8589
// CHECK: firrtl.propassign %[[bb_0_field]], %[[bb_0]]
8690
// CHECK: }
8791

8892
// CHECK: emit.file "BlackBoxes.txt" {
8993
// CHECK-NEXT: sv.verbatim "
90-
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}\0A
94+
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}.bb\0A
9195
// CHECK-SAME: symbols = [
9296
// CHECK-SAME: @DUTModule
9397
// CHECK-SAME: #hw.innerNameRef<@DUTModule::[[WRAPPER_SYM]]>
@@ -185,8 +189,8 @@ firrtl.circuit "ExtractBlackBoxesSimple2" attributes {annotations = [{class = "f
185189
}
186190
// CHECK: emit.file "BlackBoxes.txt" {
187191
// CHECK-NEXT: sv.verbatim "
188-
// CHECK-SAME{LITERAL}: prefix_0 -> {{0}}.{{1}}\0A
189-
// CHECK-SAME{LITERAL}: prefix_1 -> {{0}}.{{1}}\0A
192+
// CHECK-SAME{LITERAL}: prefix_0 -> {{0}}.{{1}}.bb2\0A
193+
// CHECK-SAME{LITERAL}: prefix_1 -> {{0}}.{{1}}.bb\0A
190194
// CHECK-SAME: symbols = [
191195
// CHECK-SAME: @DUTModule
192196
// CHECK-SAME: @DUTModule::[[WRAPPER_SYM]]
@@ -289,8 +293,8 @@ firrtl.circuit "ExtractBlackBoxesIntoDUTSubmodule" {
289293
}
290294
// CHECK: emit.file "BlackBoxes.txt" {
291295
// CHECK-NEXT: sv.verbatim "
292-
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}\0A
293-
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}\0A
296+
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}.bb2\0A
297+
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}.bb1\0A
294298
// CHECK-SAME: symbols = [
295299
// CHECK-SAME: @DUTModule
296300
// CHECK-SAME: @DUTModule::[[WRAPPER_SYM]]
@@ -486,7 +490,7 @@ firrtl.circuit "ExtractClockGatesSimple" attributes {annotations = [{class = "si
486490
}
487491
// CHECK: emit.file "ClockGates.txt" {
488492
// CHECK-NEXT: sv.verbatim "
489-
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}\0A
493+
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.gate\0A
490494
// CHECK-SAME: symbols = [
491495
// CHECK-SAME: @DUTModule
492496
// CHECK-SAME: ]
@@ -563,8 +567,8 @@ firrtl.circuit "ExtractClockGatesMixed" attributes {annotations = [{class = "sif
563567
}
564568
// CHECK: emit.file "ClockGates.txt" {
565569
// CHECK-NEXT: sv.verbatim "
566-
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}\0A
567-
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}\0A
570+
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.gate\0A
571+
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.gate\0A
568572
// CHECK-SAME: symbols = [
569573
// CHECK-SAME: @DUTModule
570574
// CHECK-SAME: @DUTModule::@inst
@@ -611,25 +615,25 @@ firrtl.circuit "ExtractClockGatesComposed" attributes {annotations = [
611615
%sifive_metadata = firrtl.object @SiFive_Metadata()
612616
// CHECK: firrtl.object @SiFive_Metadata(
613617
// CHECK-SAME: out extractedInstances_field0: !firrtl.class<@ExtractInstancesMetadata
614-
// CHECK-SAME: (out mem_wiring_0_field0: !firrtl.class<@ExtractInstancesSchema(in name_in: !firrtl.string, out name: !firrtl.string, in path_in: !firrtl.path, out path: !firrtl.path, in filename_in: !firrtl.string, out filename: !firrtl.string)>
615-
// CHECK-SAME: out clock_gate_0_field1: !firrtl.class<@ExtractInstancesSchema(in name_in: !firrtl.string, out name: !firrtl.string, in path_in: !firrtl.path, out path: !firrtl.path, in filename_in: !firrtl.string, out filename: !firrtl.string)>
616-
// CHECK-SAME: out clock_gate_1_field3: !firrtl.class<@ExtractInstancesSchema(in name_in: !firrtl.string, out name: !firrtl.string, in path_in: !firrtl.path, out path: !firrtl.path, in filename_in: !firrtl.string, out filename: !firrtl.string)>)>)
618+
// CHECK-SAME: (out mem_wiring_0_field0: !firrtl.class<@ExtractInstancesSchema(in name_in: !firrtl.string, out name: !firrtl.string, in path_in: !firrtl.path, out path: !firrtl.path, in filename_in: !firrtl.string, out filename: !firrtl.string, in inst_name_in: !firrtl.string, out inst_name: !firrtl.string)>
619+
// CHECK-SAME: out clock_gate_0_field1: !firrtl.class<@ExtractInstancesSchema(in name_in: !firrtl.string, out name: !firrtl.string, in path_in: !firrtl.path, out path: !firrtl.path, in filename_in: !firrtl.string, out filename: !firrtl.string, in inst_name_in: !firrtl.string, out inst_name: !firrtl.string)>
620+
// CHECK-SAME: out clock_gate_1_field3: !firrtl.class<@ExtractInstancesSchema(in name_in: !firrtl.string, out name: !firrtl.string, in path_in: !firrtl.path, out path: !firrtl.path, in filename_in: !firrtl.string, out filename: !firrtl.string, in inst_name_in: !firrtl.string, out inst_name: !firrtl.string)>)>)
617621
%0 = firrtl.object.anyref_cast %sifive_metadata : !firrtl.class<@SiFive_Metadata()>
618622
firrtl.propassign %metadataObj, %0 : !firrtl.anyref
619623
}
620624
firrtl.class @SiFive_Metadata() {}
621625

622626
// CHECK: emit.file "SeqMems.txt" {
623627
// CHECK-NEXT: sv.verbatim "
624-
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}\0A
628+
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}.mem_ext\0A
625629
// CHECK-SAME: symbols = [
626630
// CHECK-SAME: @DUTModule
627631
// CHECK-SAME: ]
628632

629633
// CHECK: emit.file "ClockGates.txt" {
630634
// CHECK-NEXT: sv.verbatim "
631-
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}\0A
632-
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}\0A
635+
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.gate\0A
636+
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.gate\0A
633637
// CHECK-SAME: symbols = [
634638
// CHECK-SAME: @DUTModule
635639
// CHECK-SAME: #hw.innerNameRef<@DUTModule::[[SYM0]]>
@@ -661,7 +665,7 @@ firrtl.circuit "ExtractSeqMemsSimple2" attributes {annotations = [{class = "sifi
661665
}
662666
// CHECK: emit.file "SeqMems.txt" {
663667
// CHECK-NEXT: sv.verbatim "
664-
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}.{{1}}\0A
668+
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}.{{1}}.mem_ext\0A
665669
// CHECK-SAME: symbols = [
666670
// CHECK-SAME: @DUTModule
667671
// CHECK-SAME: @DUTModule::[[MEM_SYM]]
@@ -725,8 +729,8 @@ firrtl.circuit "InstSymConflict" {
725729
}
726730
// CHECK: emit.file "BlackBoxes.txt" {
727731
// CHECK-NEXT: sv.verbatim "
728-
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}\0A
729-
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{2}}\0A
732+
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}.bb\0A
733+
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{2}}.bb\0A
730734
// CHECK-SAME: symbols = [
731735
// CHECK-SAME: @DUTModule
732736
// CHECK-SAME: #hw.innerNameRef<@DUTModule::@mod1>

0 commit comments

Comments
 (0)