From 19bf11a8b01a4e16b23ab40ed3d62fc9d995bfb0 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 21 Nov 2024 13:37:50 -0800 Subject: [PATCH 1/3] [ExtractInstances] Append original instance name to path in metadata --- .../FIRRTL/Transforms/ExtractInstances.cpp | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 15189009085f..56bbe5ced281 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,7 +160,8 @@ 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; @@ -177,7 +179,7 @@ void ExtractInstancesPass::runOnOperation() { extractionPaths.clear(); originalInstanceParents.clear(); extractedInstances.clear(); - instPrefices.clear(); + instPrefixNamesPair.clear(); moduleNamespaces.clear(); circuitNamespace.clear(); circuitNamespace.add(circuitOp); @@ -495,8 +497,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 +639,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 +889,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 +1053,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 +1094,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 +1122,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. @@ -1159,9 +1170,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("ExtractInstancesSchema", portFields, portsType); From a57547b3ba5c349026c2ed1da8bf55046d33f5f6 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 21 Nov 2024 13:38:01 -0800 Subject: [PATCH 2/3] Update lit tests --- .../extract-instances-inject-dut-hier.mlir | 4 +- test/Dialect/FIRRTL/extract-instances.mlir | 40 ++++++++++--------- 2 files changed, 24 insertions(+), 20 deletions(-) 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> From ec936aba12edde160f4ecfbc374a44229fc73ce2 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Mon, 20 Jan 2025 22:24:32 -0800 Subject: [PATCH 3/3] Cache string types --- lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 56bbe5ced281..2448610576a5 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -165,6 +165,7 @@ struct ExtractInstancesPass /// 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 @@ -186,6 +187,9 @@ void ExtractInstancesPass::runOnOperation() { 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. @@ -1168,10 +1172,10 @@ 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::get(context) // instance name + stringType, // name + pathType, // extracted instance path + stringType, // filename + stringType // instance name }; StringRef portFields[] = {"name", "path", "filename", "inst_name"};