Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ExtractInstances] Append original instance name to path in metadata #7872

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+51 −35
Open

[ExtractInstances] Append original instance name to path in metadata #7872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47bf2a8
[ExtractInstances] Append original instance name to path in metadata
prithayan Nov 21, 2024
8d58bdd
Update lit tests
prithayan Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 27 additions & 15 deletions lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,18 @@ struct ExtractInstancesPass
/// hierarchy, but before being grouped into an optional submodule.
SmallVector<std::pair<InstanceOp, ExtractionInfo>> extractedInstances;

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

/// The current circuit namespace valid within the call to `runOnOperation`.
CircuitNamespace circuitNamespace;
/// Cached module namespaces.
DenseMap<Operation *, hw::InnerSymbolNamespace> moduleNamespaces;
/// The metadata class ops.
ClassOp extractMetadataClass, schemaClass;
const unsigned prefixNameFieldId = 0, pathFieldId = 2, fileNameFieldId = 4;
const unsigned prefixNameFieldId = 0, pathFieldId = 2, fileNameFieldId = 4,
instNameFieldId = 6;
/// Cache of the inner ref to the new instances created. Will be used to
/// create a path to the instance
DenseMap<InnerRefAttr, InstanceOp> innerRefToInstances;
Expand All @@ -187,7 +189,7 @@ void ExtractInstancesPass::runOnOperation() {
extractionPaths.clear();
originalInstanceParents.clear();
extractedInstances.clear();
instPrefices.clear();
instPrefixNamesPair.clear();
moduleNamespaces.clear();
circuitNamespace.clear();
circuitNamespace.add(circuitOp);
Expand Down Expand Up @@ -511,8 +513,10 @@ void ExtractInstancesPass::extractInstances() {
// of the pass does, which would group instances to be extracted by prefix
// and then iterate over them with the index in the group being used as `N`.
StringRef prefix;
auto &instPrefixEntry = instPrefixNamesPair[inst];
instPrefixEntry.second = inst.getInstanceNameAttr();
if (!info.prefix.empty()) {
auto &prefixSlot = instPrefices[inst];
auto &prefixSlot = instPrefixEntry.first;
if (prefixSlot.empty()) {
auto idx = prefixUniqueIDs[info.prefix]++;
(Twine(info.prefix) + "_" + Twine(idx)).toVector(prefixSlot);
Expand Down Expand Up @@ -651,13 +655,13 @@ void ExtractInstancesPass::extractInstances() {
// new instance we create inherit the wiring prefix, and all additional
// new instances (e.g. through multiple instantiation of the parent) will
// pick a new prefix.
auto oldPrefix = instPrefices.find(inst);
if (oldPrefix != instPrefices.end()) {
LLVM_DEBUG(llvm::dbgs()
<< " - Reusing prefix `" << oldPrefix->second << "`\n");
auto oldPrefix = instPrefixNamesPair.find(inst);
if (oldPrefix != instPrefixNamesPair.end()) {
LLVM_DEBUG(llvm::dbgs() << " - Reusing prefix `"
<< oldPrefix->second.first << "`\n");
auto newPrefix = std::move(oldPrefix->second);
instPrefices.erase(oldPrefix);
instPrefices.insert({newInst, newPrefix});
instPrefixNamesPair.erase(oldPrefix);
instPrefixNamesPair.insert({newInst, newPrefix});
}

// Inherit the old instance's extraction path.
Expand Down Expand Up @@ -901,7 +905,7 @@ void ExtractInstancesPass::groupInstances() {
ports.clear();
for (auto inst : insts) {
// Determine the ports for the wrapper.
StringRef prefix(instPrefices[inst]);
StringRef prefix(instPrefixNamesPair[inst].first);
unsigned portNum = inst.getNumResults();
for (unsigned portIdx = 0; portIdx < portNum; ++portIdx) {
auto name = inst.getPortNameStr(portIdx);
Expand Down Expand Up @@ -1065,7 +1069,8 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) {
auto file = getOrCreateFile(fileName);
auto builder = OpBuilder::atBlockEnd(file.getBody());
for (auto inst : insts) {
StringRef prefix(instPrefices[inst]);
StringRef prefix(instPrefixNamesPair[inst].first);
StringAttr origInstName(instPrefixNamesPair[inst].second);
if (prefix.empty()) {
LLVM_DEBUG(llvm::dbgs() << " - Skipping `" << inst.getName()
<< "` since it has no extraction prefix\n");
Expand Down Expand Up @@ -1105,6 +1110,11 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) {
builderOM.create<StringConstantOp>(builder.getStringAttr(fileName));
builderOM.create<PropAssignOp>(fFile, fileNameOp);

auto finstName =
builderOM.create<ObjectSubfieldOp>(object, instNameFieldId);
auto instNameOp = builderOM.create<StringConstantOp>(origInstName);
builderOM.create<PropAssignOp>(finstName, instNameOp);

// Now add this to the output field of the class.
classFields.emplace_back(object, prefix + "_field");
}
Expand All @@ -1128,6 +1138,7 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) {
os << ".";
addSymbol(sym);
}
os << "." << origInstName.getValue();
// The final instance name is excluded as this does not provide useful
// additional information and could conflict with a name inside the final
// module.
Expand Down Expand Up @@ -1175,9 +1186,10 @@ void ExtractInstancesPass::createSchema() {
mlir::Type portsType[] = {
StringType::get(context), // name
PathType::get(context), // extracted instance path
StringType::get(context) // filename
StringType::get(context), // filename
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please call get once and reuse the value. Type uniquing has costs we don't need to pay here.

StringType::get(context) // instance name
};
StringRef portFields[] = {"name", "path", "filename"};
StringRef portFields[] = {"name", "path", "filename", "inst_name"};

schemaClass = builderOM.create<ClassOp>("ExtractInstancesSchema", portFields,
portsType);
Expand Down
4 changes: 2 additions & 2 deletions test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ firrtl.circuit "ExtractClockGatesMultigrouping" attributes {annotations = [{clas
}
// CHECK: emit.file "ClockGates.txt" {
// CHECK-NEXT: sv.verbatim
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.{{1}}.{{2}}\0A
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.{{3}}\0A
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.{{1}}.{{2}}.gate\0A
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.{{3}}.gate\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: #hw.innerNameRef<@DUTModule::[[INJMOD_SYM]]>
Expand Down
40 changes: 22 additions & 18 deletions test/Dialect/FIRRTL/extract-instances.mlir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi
// CHECK: firrtl.propassign %[[extractedInstances_field_0]], %extract_instances_metadata : !firrtl.class<@ExtractInstancesMetadata
// CHECK: }

// 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) {
// 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) {
// CHECK: firrtl.propassign %name, %name_in : !firrtl.string
// CHECK: firrtl.propassign %path, %path_in : !firrtl.path
// CHECK: firrtl.propassign %filename, %filename_in : !firrtl.string
// CHECK: firrtl.propassign %inst_name, %inst_name_in : !firrtl.string
// CHECK: }

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

// CHECK: emit.file "BlackBoxes.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}.bb\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: #hw.innerNameRef<@DUTModule::[[WRAPPER_SYM]]>
Expand Down Expand Up @@ -185,8 +189,8 @@ firrtl.circuit "ExtractBlackBoxesSimple2" attributes {annotations = [{class = "f
}
// CHECK: emit.file "BlackBoxes.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: prefix_0 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: prefix_1 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: prefix_0 -> {{0}}.{{1}}.bb2\0A
// CHECK-SAME{LITERAL}: prefix_1 -> {{0}}.{{1}}.bb\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: @DUTModule::[[WRAPPER_SYM]]
Expand Down Expand Up @@ -289,8 +293,8 @@ firrtl.circuit "ExtractBlackBoxesIntoDUTSubmodule" {
}
// CHECK: emit.file "BlackBoxes.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{1}}.bb2\0A
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}.bb1\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: @DUTModule::[[WRAPPER_SYM]]
Expand Down Expand Up @@ -486,7 +490,7 @@ firrtl.circuit "ExtractClockGatesSimple" attributes {annotations = [{class = "si
}
// CHECK: emit.file "ClockGates.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}\0A
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.gate\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: ]
Expand Down Expand Up @@ -563,8 +567,8 @@ firrtl.circuit "ExtractClockGatesMixed" attributes {annotations = [{class = "sif
}
// CHECK: emit.file "ClockGates.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}\0A
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.gate\0A
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.gate\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: @DUTModule::@inst
Expand Down Expand Up @@ -611,25 +615,25 @@ firrtl.circuit "ExtractClockGatesComposed" attributes {annotations = [
%sifive_metadata = firrtl.object @SiFive_Metadata()
// CHECK: firrtl.object @SiFive_Metadata(
// CHECK-SAME: out extractedInstances_field0: !firrtl.class<@ExtractInstancesMetadata
// 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)>
// 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)>
// 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)>)>)
// 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)>
// 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)>
// 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)>)>)
%0 = firrtl.object.anyref_cast %sifive_metadata : !firrtl.class<@SiFive_Metadata()>
firrtl.propassign %metadataObj, %0 : !firrtl.anyref
}
firrtl.class @SiFive_Metadata() {}

// CHECK: emit.file "SeqMems.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}\0A
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}.mem_ext\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: ]

// CHECK: emit.file "ClockGates.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}\0A
// CHECK-SAME{LITERAL}: clock_gate_0 -> {{0}}.{{1}}.gate\0A
// CHECK-SAME{LITERAL}: clock_gate_1 -> {{0}}.gate\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: #hw.innerNameRef<@DUTModule::[[SYM0]]>
Expand Down Expand Up @@ -661,7 +665,7 @@ firrtl.circuit "ExtractSeqMemsSimple2" attributes {annotations = [{class = "sifi
}
// CHECK: emit.file "SeqMems.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: mem_wiring_0 -> {{0}}.{{1}}.mem_ext\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: @DUTModule::[[MEM_SYM]]
Expand Down Expand Up @@ -725,8 +729,8 @@ firrtl.circuit "InstSymConflict" {
}
// CHECK: emit.file "BlackBoxes.txt" {
// CHECK-NEXT: sv.verbatim "
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}\0A
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{2}}\0A
// CHECK-SAME{LITERAL}: bb_1 -> {{0}}.{{1}}.bb\0A
// CHECK-SAME{LITERAL}: bb_0 -> {{0}}.{{2}}.bb\0A
// CHECK-SAME: symbols = [
// CHECK-SAME: @DUTModule
// CHECK-SAME: #hw.innerNameRef<@DUTModule::@mod1>
Expand Down
Loading