From 77553d7f263a95bf4d389565c44176723f6e48c8 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Tue, 21 Jan 2025 06:01:53 -0800 Subject: [PATCH] [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. --- .../FIRRTL/Transforms/ExtractInstances.cpp | 50 ++++++++++++------- .../extract-instances-inject-dut-hier.mlir | 4 +- test/Dialect/FIRRTL/extract-instances.mlir | 40 ++++++++------- 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 15189009085f..2448610576a5 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -150,8 +150,9 @@ struct ExtractInstancesPass /// hierarchy, but before being grouped into an optional submodule. SmallVector> extractedInstances; - // The uniquified wiring prefix for each instance. - DenseMap> instPrefices; + // The uniquified wiring prefix and original name for each instance. + DenseMap, StringAttr>> + instPrefixNamesPair; /// The current circuit namespace valid within the call to `runOnOperation`. CircuitNamespace circuitNamespace; @@ -159,10 +160,12 @@ struct ExtractInstancesPass DenseMap 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 innerRefToInstances; + Type stringType, pathType; }; } // end anonymous namespace @@ -177,13 +180,16 @@ void ExtractInstancesPass::runOnOperation() { extractionPaths.clear(); originalInstanceParents.clear(); extractedInstances.clear(); - instPrefices.clear(); + instPrefixNamesPair.clear(); moduleNamespaces.clear(); circuitNamespace.clear(); circuitNamespace.add(circuitOp); innerRefToInstances.clear(); extractMetadataClass = {}; schemaClass = {}; + auto *context = circuitOp->getContext(); + stringType = StringType::get(context); + pathType = PathType::get(context); // Walk the IR and gather all the annotations relevant for extraction that // appear on instances and the instantiated modules. @@ -495,8 +501,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); @@ -635,13 +643,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. @@ -885,7 +893,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); @@ -1049,7 +1057,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"); @@ -1089,6 +1098,11 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { builderOM.create(builder.getStringAttr(fileName)); builderOM.create(fFile, fileNameOp); + auto finstName = + builderOM.create(object, instNameFieldId); + auto instNameOp = builderOM.create(origInstName); + builderOM.create(finstName, instNameOp); + // Now add this to the output field of the class. classFields.emplace_back(object, prefix + "_field"); } @@ -1112,6 +1126,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. @@ -1157,11 +1172,12 @@ void ExtractInstancesPass::createSchema() { auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( unknownLoc, circuitOp.getBodyBlock()); mlir::Type portsType[] = { - StringType::get(context), // name - PathType::get(context), // extracted instance path - StringType::get(context) // filename + stringType, // name + pathType, // extracted instance path + stringType, // filename + stringType // instance name }; - StringRef portFields[] = {"name", "path", "filename"}; + StringRef portFields[] = {"name", "path", "filename", "inst_name"}; schemaClass = builderOM.create("ExtractInstancesSchema", portFields, portsType); diff --git a/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir b/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir index 3b47ca8933b4..dde85db59a8d 100644 --- a/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir +++ b/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir @@ -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]]> diff --git a/test/Dialect/FIRRTL/extract-instances.mlir b/test/Dialect/FIRRTL/extract-instances.mlir index 9bc6f23d5888..deeb1c9e47e8 100644 --- a/test/Dialect/FIRRTL/extract-instances.mlir +++ b/test/Dialect/FIRRTL/extract-instances.mlir @@ -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 @@ -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]]> @@ -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]] @@ -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]] @@ -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: ] @@ -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 @@ -611,9 +615,9 @@ 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 } @@ -621,15 +625,15 @@ firrtl.circuit "ExtractClockGatesComposed" attributes {annotations = [ // 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]]> @@ -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]] @@ -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>