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.

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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
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