From 4bc6fd35ec62fb658c4b84461f13910efdebcb0b Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Fri, 4 Oct 2024 14:54:04 -0700 Subject: [PATCH 1/9] [ExtractInstances] Add the extract instances metadata to OM dialect --- include/circt/Dialect/FIRRTL/FIRRTLUtils.h | 17 ++- lib/Dialect/FIRRTL/FIRRTLUtils.cpp | 50 +++++++ .../Transforms/CreateSiFiveMetadata.cpp | 27 ---- .../FIRRTL/Transforms/ExtractInstances.cpp | 122 +++++++++++++++++- 4 files changed, 180 insertions(+), 36 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h index 9dc03475d2a0..c8625647dccb 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h @@ -228,8 +228,7 @@ inline FIRRTLBaseType getBaseType(Type type) { } /// Get base type if isa<> the requested type, else null. -template -inline T getBaseOfType(Type type) { +template inline T getBaseOfType(Type type) { return dyn_cast_or_null(getBaseType(type)); } @@ -322,6 +321,20 @@ static ResultTy transformReduce(MLIRContext *context, RangeTy &&r, /// Truncate `a` to the common prefix of `a` and `b`. void makeCommonPrefix(SmallString<64> &a, StringRef b); +//===----------------------------------------------------------------------===// +// OM dialect utilities +//===----------------------------------------------------------------------===// + +/// Create a ClassOp, with the specified fieldNames and fieldTypes as ports. The +/// output property is set from the input property port. +ClassOp buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, Twine name, + ArrayRef fieldNames, + ArrayRef fieldTypes); + +/// Add the tracker annotation to the op and get a PathOp to the op. +PathOp createPathRef(Operation *op, hw::HierPathOp nla, + mlir::ImplicitLocOpBuilder &builderOM); + } // namespace firrtl } // namespace circt diff --git a/lib/Dialect/FIRRTL/FIRRTLUtils.cpp b/lib/Dialect/FIRRTL/FIRRTLUtils.cpp index 3a97d86e32b9..2230bf5644ab 100644 --- a/lib/Dialect/FIRRTL/FIRRTLUtils.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLUtils.cpp @@ -1075,3 +1075,53 @@ void circt::firrtl::makeCommonPrefix(SmallString<64> &a, StringRef b) { while (!a.empty() && !a.ends_with(sep)) a.pop_back(); } + +ClassOp circt::firrtl::buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, + Twine name, + ArrayRef fieldNames, + ArrayRef fieldTypes) { + SmallVector ports; + for (auto [fieldName, fieldType] : llvm::zip(fieldNames, fieldTypes)) { + ports.emplace_back(odsBuilder.getStringAttr(fieldName + "_in"), fieldType, + Direction::In); + ports.emplace_back(odsBuilder.getStringAttr(fieldName), fieldType, + Direction::Out); + } + + ClassOp classOp = + odsBuilder.create(loc, odsBuilder.getStringAttr(name), ports); + Block *body = classOp.getBodyBlock(); + auto prevLoc = odsBuilder.saveInsertionPoint(); + odsBuilder.setInsertionPointToEnd(body); + auto args = body->getArguments(); + for (unsigned i = 0, e = ports.size(); i != e; i += 2) + odsBuilder.create(loc, args[i + 1], args[i]); + + odsBuilder.restoreInsertionPoint(prevLoc); + + return classOp; +} + +PathOp circt::firrtl::createPathRef(Operation *op, hw::HierPathOp nla, + mlir::ImplicitLocOpBuilder &builderOM) { + + auto *context = op->getContext(); + auto id = DistinctAttr::create(UnitAttr::get(context)); + TargetKind kind = TargetKind::Reference; + // If op is null, then create an empty path. + if (op) { + NamedAttrList fields; + fields.append("id", id); + fields.append("class", StringAttr::get(context, "circt.tracker")); + if (nla) + fields.append("circt.nonlocal", mlir::FlatSymbolRefAttr::get(nla)); + AnnotationSet annos(op); + annos.addAnnotations(DictionaryAttr::get(context, fields)); + annos.applyToOperation(op); + if (isa(op)) + kind = TargetKind::Instance; + } + + // Create the path operation. + return builderOM.create(kind, id); +} diff --git a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp index 6c4dc768db5c..8b346b1cbe63 100644 --- a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp +++ b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp @@ -75,33 +75,6 @@ struct ObjectModelIR { return builderOM.create(kind, id); } - // Create a ClassOp, with the specified fieldNames and fieldTypes as ports. - // The output property is set from the input property port. - ClassOp buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, Twine name, - ArrayRef fieldNames, - ArrayRef fieldTypes) { - SmallVector ports; - for (auto [fieldName, fieldType] : llvm::zip(fieldNames, fieldTypes)) { - ports.emplace_back(odsBuilder.getStringAttr(fieldName + "_in"), fieldType, - Direction::In); - ports.emplace_back(odsBuilder.getStringAttr(fieldName), fieldType, - Direction::Out); - } - - ClassOp classOp = - odsBuilder.create(loc, odsBuilder.getStringAttr(name), ports); - Block *body = classOp.getBodyBlock(); - auto prevLoc = odsBuilder.saveInsertionPoint(); - odsBuilder.setInsertionPointToEnd(body); - auto args = body->getArguments(); - for (unsigned i = 0, e = ports.size(); i != e; i += 2) - odsBuilder.create(loc, args[i + 1], args[i]); - - odsBuilder.restoreInsertionPoint(prevLoc); - - return classOp; - } - void createMemorySchema() { auto unknownLoc = mlir::UnknownLoc::get(context); diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 2d961fc101c8..a61c106c368f 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -14,6 +14,7 @@ #include "circt/Dialect/Emit/EmitOps.h" #include "circt/Dialect/FIRRTL/AnnotationDetails.h" +#include "circt/Dialect/FIRRTL/FIRRTLAnnotationHelper.h" #include "circt/Dialect/FIRRTL/FIRRTLAnnotations.h" #include "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" #include "circt/Dialect/FIRRTL/FIRRTLOps.h" @@ -73,7 +74,8 @@ struct ExtractInstancesPass void collectAnno(InstanceOp inst, Annotation anno); void extractInstances(); void groupInstances(); - void createTraceFiles(); + void createTraceFiles(ClassOp &sifiveMetadata); + void createSchema(); /// Get the cached namespace for a module. hw::InnerSymbolNamespace &getModuleNamespace(FModuleLike module) { @@ -114,7 +116,9 @@ struct ExtractInstancesPass bool anythingChanged; bool anyFailures; + CircuitOp circtOp; InstanceGraph *instanceGraph = nullptr; + SymbolTable *symbolTable = nullptr; /// The modules in the design that are annotated with one or more annotations /// relevant for instance extraction. @@ -159,12 +163,17 @@ struct ExtractInstancesPass CircuitNamespace circuitNamespace; /// Cached module namespaces. DenseMap moduleNamespaces; + /// The metadata class ops. + ClassOp extractMetadataClass, schemaClass; + /// Cache of the inner ref to the new instances created. Will be used to + /// create a path to the instance + DenseMap innerRefToInstances; }; } // end anonymous namespace /// Emit the annotated source code for black boxes in a circuit. void ExtractInstancesPass::runOnOperation() { - CircuitOp circuitOp = getOperation(); + circtOp = getOperation(); anythingChanged = false; anyFailures = false; annotatedModules.clear(); @@ -180,11 +189,12 @@ void ExtractInstancesPass::runOnOperation() { instPrefices.clear(); moduleNamespaces.clear(); circuitNamespace.clear(); - circuitNamespace.add(circuitOp); + circuitNamespace.add(circtOp); // Walk the IR and gather all the annotations relevant for extraction that // appear on instances and the instantiated modules. instanceGraph = &getAnalysis(); + symbolTable = &getAnalysis(); collectAnnos(); if (anyFailures) return signalPassFailure(); @@ -199,8 +209,16 @@ void ExtractInstancesPass::runOnOperation() { if (anyFailures) return signalPassFailure(); + ClassOp sifiveMetadata; + for (ClassOp classOp : circtOp.getOps()) { + if (classOp.getSymNameAttr().getValue().compare("SiFive_Metadata")) + continue; + sifiveMetadata = classOp; + break; + } + // Generate the trace files that list where each instance was extracted from. - createTraceFiles(); + createTraceFiles(sifiveMetadata); if (anyFailures) return signalPassFailure(); @@ -608,6 +626,8 @@ void ExtractInstancesPass::extractInstances() { auto newParent = oldParentInst->getParentOfType(); LLVM_DEBUG(llvm::dbgs() << "- Updating " << oldParentInst << "\n"); auto newParentInst = oldParentInst.cloneAndInsertPorts(newPorts); + if (newParentInst.getInnerSymAttr()) + innerRefToInstances[getInnerRefTo(newParentInst)] = newParentInst; // Migrate connections to existing ports. for (unsigned portIdx = 0; portIdx < numParentPorts; ++portIdx) @@ -633,6 +653,8 @@ void ExtractInstancesPass::extractInstances() { ImplicitLocOpBuilder builder(inst.getLoc(), newParentInst); builder.setInsertionPointAfter(newParentInst); builder.insert(newInst); + if (newParentInst.getInnerSymAttr()) + innerRefToInstances[getInnerRefTo(newInst)] = newInst; for (unsigned portIdx = 0; portIdx < numInstPorts; ++portIdx) { auto dst = newInst.getResult(portIdx); auto src = newParentInst.getResult(numParentPorts + portIdx); @@ -659,7 +681,9 @@ void ExtractInstancesPass::extractInstances() { // Inherit the old instance's extraction path. extractionPaths.try_emplace(newInst); // (create entry first) auto &extractionPath = (extractionPaths[newInst] = extractionPaths[inst]); - extractionPath.push_back(getInnerRefTo(newParentInst)); + auto instInnerRef = getInnerRefTo(newParentInst); + innerRefToInstances[instInnerRef] = newParentInst; + extractionPath.push_back(instInnerRef); originalInstanceParents.try_emplace(newInst); // (create entry first) originalInstanceParents[newInst] = originalInstanceParents[inst]; // Record the Nonlocal annotations that need to be applied to the new @@ -819,7 +843,6 @@ void ExtractInstancesPass::extractInstances() { // No update to NLATable required, since it will be deleted from the // parent, and it should already exist in the new parent module. - continue; } AnnotationSet newInstAnnos(newInst); newInstAnnos.addAnnotations(newInstNonlocalAnnos); @@ -999,7 +1022,7 @@ void ExtractInstancesPass::groupInstances() { /// Generate trace files, which are plain text metadata files that list the /// hierarchical path where each instance was extracted from. The file lists one /// instance per line in the form ` -> `. -void ExtractInstancesPass::createTraceFiles() { +void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { LLVM_DEBUG(llvm::dbgs() << "\nGenerating trace files\n"); // Group the extracted instances by their trace file name. @@ -1011,7 +1034,10 @@ void ExtractInstancesPass::createTraceFiles() { // Generate the trace files. SmallVector symbols; SmallDenseMap symbolIndices; + if (sifiveMetadataClass && !extractMetadataClass) + createSchema(); + HierPathCache pathCache(circtOp, *symbolTable); for (auto &[fileName, insts] : instsByTraceFile) { LLVM_DEBUG(llvm::dbgs() << "- " << fileName << "\n"); std::string buffer; @@ -1051,6 +1077,38 @@ void ExtractInstancesPass::createTraceFiles() { << " - " << prefix << ": " << inst.getName() << "\n"); os << prefix << " -> "; + if (sifiveMetadataClass) { + // Create the entry for this extracted instance in the metadata class. + auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( + inst.getLoc(), extractMetadataClass.getBodyBlock()); + auto prefixName = builderOM.create(prefix); + auto object = builderOM.create(schemaClass, prefix); + auto fPrefix = builderOM.create(object, 0); + builderOM.create(fPrefix, prefixName); + + auto targetInstance = innerRefToInstances[path.front()]; + SmallVector pathOpAttr(llvm::reverse(path)); + auto nla = pathCache.getOpFor( + ArrayAttr::get(circtOp->getContext(), pathOpAttr)); + + auto pathOp = createPathRef(targetInstance, nla, builderOM); + builderOM.create( + builderOM.create(object, 2), pathOp); + builderOM.create( + builderOM.create(object, 4), + builderOM.create( + builder.getStringAttr(fileName))); + + // Now add this to the output field of the class. + auto portIndex = extractMetadataClass.getNumPorts(); + SmallVector> newPorts = { + {portIndex, PortInfo(builderOM.getStringAttr(prefix + "_field"), + object.getType(), Direction::Out)}}; + extractMetadataClass.insertPorts(newPorts); + auto blockarg = extractMetadataClass.getBodyBlock()->addArgument( + object.getType(), inst->getLoc()); + builderOM.create(blockarg, object); + } // HACK: To match the Scala implementation, we strip all non-DUT modules // from the path and make the path look like it's rooted at the first DUT // module (so `TestHarness.dut.foo.bar` becomes `DUTModule.foo.bar`). @@ -1075,6 +1133,35 @@ void ExtractInstancesPass::createTraceFiles() { // module. os << "\n"; } + if (sifiveMetadataClass) { + auto unknownLoc = UnknownLoc::get(circtOp->getContext()); + auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( + unknownLoc, sifiveMetadataClass.getBodyBlock()); + auto addPort = [&](Value obj, StringRef fieldName) { + auto portIndex = sifiveMetadataClass.getNumPorts(); + SmallVector> newPorts = { + {portIndex, PortInfo(builder.getStringAttr(fieldName + "_field_" + + Twine(portIndex)), + obj.getType(), Direction::Out)}}; + sifiveMetadataClass.insertPorts(newPorts); + auto blockarg = sifiveMetadataClass.getBodyBlock()->addArgument( + obj.getType(), unknownLoc); + builderOM.create(blockarg, obj); + }; + addPort(builderOM.create( + extractMetadataClass, + builder.getStringAttr("extract_instances_metadata")), + "extractedInstances"); + auto *node = instanceGraph->lookup(sifiveMetadataClass); + assert(node && node->hasOneUse()); + Operation *metadataObj = (*node->usesBegin())->getInstance(); + assert(isa(metadataObj)); + builderOM.setInsertionPoint(metadataObj); + auto newObj = builderOM.create( + sifiveMetadataClass, builder.getStringAttr("sifive_metadata")); + metadataObj->replaceAllUsesWith(newObj); + metadataObj->remove(); + } // Put the information in a verbatim operation. builder.create(builder.getUnknownLoc(), buffer, @@ -1082,6 +1169,27 @@ void ExtractInstancesPass::createTraceFiles() { } } +void ExtractInstancesPass::createSchema() { + + auto *context = circtOp->getContext(); + auto unknownLoc = mlir::UnknownLoc::get(context); + auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( + unknownLoc, circtOp.getBodyBlock()); + mlir::Type portsType[] = { + StringType::get(context), // name + PathType::get(context), // extracted instance path + StringType::get(context) // filename + }; + StringRef portFields[] = {"name", "path", "filename"}; + + schemaClass = buildSimpleClassOp( + builderOM, unknownLoc, "ExtractInstancesSchema", portFields, portsType); + + // Now create the class that will instantiate the schema objects. + SmallVector mports; + extractMetadataClass = builderOM.create( + builderOM.getStringAttr("ExtractInstancesMetadata"), mports); +} //===----------------------------------------------------------------------===// // Pass Creation //===----------------------------------------------------------------------===// From d1cbdc8d13f7e05c11b7c183934bcd0463a59e10 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Fri, 4 Oct 2024 14:54:16 -0700 Subject: [PATCH 2/9] Update lit tests --- .../extract-instances-inject-dut-hier.mlir | 4 +- test/Dialect/FIRRTL/extract-instances.mlir | 43 +++++++++++++++++-- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir b/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir index 1d903c36aaa9..3b47ca8933b4 100644 --- a/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir +++ b/test/Dialect/FIRRTL/extract-instances-inject-dut-hier.mlir @@ -22,8 +22,8 @@ firrtl.circuit "ExtractClockGatesMultigrouping" attributes {annotations = [{clas // CHECK: firrtl.instance inst1 sym [[INST1_SYM:@.+]] @SomeModule // CHECK-LABEL: firrtl.module private @ClockGatesGroup - // CHECK: firrtl.instance gate @EICG_wrapper - // CHECK: firrtl.instance gate @EICG_wrapper + // CHECK: firrtl.instance gate sym @sym @EICG_wrapper + // CHECK: firrtl.instance gate sym @sym_0 @EICG_wrapper // CHECK-LABEL: firrtl.module private @DUTModule firrtl.module private @DUTModule(in %clock: !firrtl.clock, in %foo_en: !firrtl.uint<1>, in %bar_en: !firrtl.uint<1>) attributes {annotations = [{class = "sifive.enterprise.firrtl.MarkDUTAnnotation"}]} { diff --git a/test/Dialect/FIRRTL/extract-instances.mlir b/test/Dialect/FIRRTL/extract-instances.mlir index 9c523284f70e..048b5e1852e8 100644 --- a/test/Dialect/FIRRTL/extract-instances.mlir +++ b/test/Dialect/FIRRTL/extract-instances.mlir @@ -31,7 +31,11 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi // CHECK-SAME: in %bb_0_out: !firrtl.uint<8> firrtl.module private @DUTModule(in %in: !firrtl.uint<8>, out %out: !firrtl.uint<8>) attributes {annotations = [{class = "sifive.enterprise.firrtl.MarkDUTAnnotation"}]} { // CHECK-NOT: firrtl.instance bb @MyBlackBox - // CHECK: %mod_in, %mod_out, %mod_bb_0_in, %mod_bb_0_out = firrtl.instance mod sym [[WRAPPER_SYM:@.+]] @BBWrapper + // CHECK: %mod_in, %mod_out, %mod_bb_0_in, %mod_bb_0_out = firrtl.instance mod + // CHECK-SAME: sym [[WRAPPER_SYM:@.+]] {annotations = + // CHECK-SAME: circt.nonlocal + // CHECK-SAME: id = distinct[0]<> + // CHECK-SAME: @BBWrapper // CHECK-NEXT: firrtl.matchingconnect %bb_0_in, %mod_bb_0_in // CHECK-NEXT: firrtl.matchingconnect %mod_bb_0_out, %bb_0_out %mod_in, %mod_out = firrtl.instance mod @BBWrapper(in in: !firrtl.uint<8>, out out: !firrtl.uint<8>) @@ -39,7 +43,7 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi firrtl.connect %mod_in, %in : !firrtl.uint<8>, !firrtl.uint<8> } // CHECK-LABEL: firrtl.module @ExtractBlackBoxesSimple - firrtl.module @ExtractBlackBoxesSimple(in %in: !firrtl.uint<8>, out %out: !firrtl.uint<8>) { + firrtl.module @ExtractBlackBoxesSimple(in %in: !firrtl.uint<8>, out %out: !firrtl.uint<8>, out %metadataObj: !firrtl.anyref) { // CHECK: %dut_in, %dut_out, %dut_bb_0_in, %dut_bb_0_out = firrtl.instance dut sym {{@.+}} @DUTModule // CHECK-NEXT: %bb_in, %bb_out = firrtl.instance bb @MyBlackBox // CHECK-NEXT: firrtl.matchingconnect %bb_in, %dut_bb_0_in @@ -47,7 +51,40 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi %dut_in, %dut_out = firrtl.instance dut @DUTModule(in in: !firrtl.uint<8>, out out: !firrtl.uint<8>) firrtl.connect %out, %dut_out : !firrtl.uint<8>, !firrtl.uint<8> firrtl.connect %dut_in, %in : !firrtl.uint<8>, !firrtl.uint<8> + %sifive_metadata = firrtl.object @SiFive_Metadata() + // CHECK: firrtl.object @SiFive_Metadata(out extractedInstances_field_0: !firrtl.class<@ExtractInstancesMetadata + %0 = firrtl.object.anyref_cast %sifive_metadata : !firrtl.class<@SiFive_Metadata()> + firrtl.propassign %metadataObj, %0 : !firrtl.anyref } + firrtl.class @SiFive_Metadata() {} + // CHECK: firrtl.class @SiFive_Metadata( + // CHECK-SAME: out %extractedInstances_field_0: !firrtl.class<@ExtractInstancesMetadata + // CHECK-SAME: { + // CHECK: %extract_instances_metadata = firrtl.object @ExtractInstancesMetadata(out bb_0_field: !firrtl.class<@ExtractInstancesSchema + // 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.propassign %name, %name_in : !firrtl.string + // CHECK: firrtl.propassign %path, %path_in : !firrtl.path + // CHECK: firrtl.propassign %filename, %filename_in : !firrtl.string + // CHECK: } + + // CHECK: firrtl.class @ExtractInstancesMetadata(out %bb_0_field: !firrtl.class<@ExtractInstancesSchema + // CHECK-SAME: { + // CHECK: %[[V0:.+]] = firrtl.string "bb_0" + // CHECK: %[[bb_0:.+]] = firrtl.object @ExtractInstancesSchema + // CHECK: %[[V1:.+]] = firrtl.object.subfield %[[bb_0]][name_in] + // CHECK: firrtl.propassign %[[V1]], %[[V0]] : !firrtl.string + // CHECK: %[[V2:.+]] = firrtl.path instance distinct[0]<> + // CHECK: %[[V3:.+]] = firrtl.object.subfield %[[bb_0]][path_in] + // CHECK: firrtl.propassign %[[V3]], %[[V2]] : !firrtl.path + // CHECK: %[[V4:.+]] = firrtl.object.subfield %[[bb_0]][filename_in] + // CHECK: %[[V5:.+]] = firrtl.string "BlackBoxes.txt" + // CHECK: firrtl.propassign %[[V4]], %[[V5]] : !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 @@ -139,7 +176,7 @@ firrtl.circuit "ExtractBlackBoxesSimple2" attributes {annotations = [{class = "f // CHECK-NEXT: %bb_in, %bb_out = firrtl.instance bb sym [[BB_SYM:@.+]] {annotations = [{class = "Old1"}, {class = "On1"}, {class = "Old2"}, {class = "On2"}]} @MyBlackBox // CHECK-NEXT: firrtl.matchingconnect %bb_in, %dut_prefix_1_in // CHECK-NEXT: firrtl.matchingconnect %dut_prefix_1_out, %bb_out - // CHECK-NEXT: %bb2_in, %bb2_out = firrtl.instance bb2 @MyBlackBox2 + // CHECK-NEXT: %bb2_in, %bb2_out = firrtl.instance bb2 sym @sym @MyBlackBox2 // CHECK-NEXT: firrtl.matchingconnect %bb2_in, %dut_prefix_0_in // CHECK-NEXT: firrtl.matchingconnect %dut_prefix_0_out, %bb2_out %dut_in, %dut_out = firrtl.instance dut sym @dut @DUTModule(in in: !firrtl.uint<8>, out out: !firrtl.uint<8>) From c55b5020ab42a6788e802d1d8a4639555444a118 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Fri, 4 Oct 2024 21:55:09 -0700 Subject: [PATCH 3/9] clang-format --- include/circt/Dialect/FIRRTL/FIRRTLUtils.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h index c8625647dccb..3c3e02d1cdbc 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h @@ -228,7 +228,8 @@ inline FIRRTLBaseType getBaseType(Type type) { } /// Get base type if isa<> the requested type, else null. -template inline T getBaseOfType(Type type) { +template +inline T getBaseOfType(Type type) { return dyn_cast_or_null(getBaseType(type)); } From 126c3d2782d9b690fe50e5119293b68abb03ffb6 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Wed, 9 Oct 2024 07:38:45 -0700 Subject: [PATCH 4/9] Address review feedback --- .../circt/Dialect/FIRRTL/FIRRTLStructure.td | 7 +++++ include/circt/Dialect/FIRRTL/FIRRTLUtils.h | 9 +----- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 28 ++++++++++++++++-- lib/Dialect/FIRRTL/FIRRTLUtils.cpp | 26 ----------------- .../Transforms/CreateSiFiveMetadata.cpp | 22 +++++++------- .../FIRRTL/Transforms/ExtractInstances.cpp | 29 +++++++++---------- 6 files changed, 59 insertions(+), 62 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td index dbd23c955ff3..93ef916d957d 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td @@ -361,6 +361,13 @@ def ClassOp : FIRRTLModuleLike<"class", [ let extraModuleClassDeclaration = [{ Block *getBodyBlock() { return &getBody().front(); } + // This builds a ClassOp, with the specified fieldNames and fieldTypes as + // ports. The output property is set from the input property port. + ClassOp static buildSimpleClassOp( + mlir::OpBuilder &odsBuilder, mlir::Location loc, mlir::Twine name, + mlir::ArrayRef fieldNames, + mlir::ArrayRef fieldTypes); + using iterator = Block::iterator; iterator begin() { return getBodyBlock()->begin(); } iterator end() { return getBodyBlock()->end(); } diff --git a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h index 3c3e02d1cdbc..3c4da7c98aad 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h @@ -228,8 +228,7 @@ inline FIRRTLBaseType getBaseType(Type type) { } /// Get base type if isa<> the requested type, else null. -template -inline T getBaseOfType(Type type) { +template inline T getBaseOfType(Type type) { return dyn_cast_or_null(getBaseType(type)); } @@ -326,12 +325,6 @@ void makeCommonPrefix(SmallString<64> &a, StringRef b); // OM dialect utilities //===----------------------------------------------------------------------===// -/// Create a ClassOp, with the specified fieldNames and fieldTypes as ports. The -/// output property is set from the input property port. -ClassOp buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, Twine name, - ArrayRef fieldNames, - ArrayRef fieldTypes); - /// Add the tracker annotation to the op and get a PathOp to the op. PathOp createPathRef(Operation *op, hw::HierPathOp nla, mlir::ImplicitLocOpBuilder &builderOM); diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 87866cc2fff3..1059c4f9cfe3 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -1991,6 +1991,31 @@ void ClassOp::print(OpAsmPrinter &p) { printClassLike(p, cast(getOperation())); } +ClassOp ClassOp::buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, + Twine name, ArrayRef fieldNames, + ArrayRef fieldTypes) { + SmallVector ports; + for (auto [fieldName, fieldType] : llvm::zip(fieldNames, fieldTypes)) { + ports.emplace_back(odsBuilder.getStringAttr(fieldName + "_in"), fieldType, + Direction::In); + ports.emplace_back(odsBuilder.getStringAttr(fieldName), fieldType, + Direction::Out); + } + + ClassOp classOp = + odsBuilder.create(loc, odsBuilder.getStringAttr(name), ports); + Block *body = classOp.getBodyBlock(); + auto prevLoc = odsBuilder.saveInsertionPoint(); + odsBuilder.setInsertionPointToEnd(body); + auto args = body->getArguments(); + for (unsigned i = 0, e = ports.size(); i != e; i += 2) + odsBuilder.create(loc, args[i + 1], args[i]); + + odsBuilder.restoreInsertionPoint(prevLoc); + + return classOp; +} + ParseResult ClassOp::parse(OpAsmParser &parser, OperationState &result) { auto hasSSAIdentifiers = true; return parseClassLike(parser, result, hasSSAIdentifiers); @@ -4594,8 +4619,7 @@ void SubtagOp::print(::mlir::OpAsmPrinter &printer) { printer << " : " << getInput().getType(); } -template -static LogicalResult verifySubfieldLike(OpTy op) { +template static LogicalResult verifySubfieldLike(OpTy op) { if (op.getFieldIndex() >= firrtl::type_cast(op.getInput().getType()) .getNumElements()) diff --git a/lib/Dialect/FIRRTL/FIRRTLUtils.cpp b/lib/Dialect/FIRRTL/FIRRTLUtils.cpp index 2230bf5644ab..6b675cec27a2 100644 --- a/lib/Dialect/FIRRTL/FIRRTLUtils.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLUtils.cpp @@ -1076,32 +1076,6 @@ void circt::firrtl::makeCommonPrefix(SmallString<64> &a, StringRef b) { a.pop_back(); } -ClassOp circt::firrtl::buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, - Twine name, - ArrayRef fieldNames, - ArrayRef fieldTypes) { - SmallVector ports; - for (auto [fieldName, fieldType] : llvm::zip(fieldNames, fieldTypes)) { - ports.emplace_back(odsBuilder.getStringAttr(fieldName + "_in"), fieldType, - Direction::In); - ports.emplace_back(odsBuilder.getStringAttr(fieldName), fieldType, - Direction::Out); - } - - ClassOp classOp = - odsBuilder.create(loc, odsBuilder.getStringAttr(name), ports); - Block *body = classOp.getBodyBlock(); - auto prevLoc = odsBuilder.saveInsertionPoint(); - odsBuilder.setInsertionPointToEnd(body); - auto args = body->getArguments(); - for (unsigned i = 0, e = ports.size(); i != e; i += 2) - odsBuilder.create(loc, args[i + 1], args[i]); - - odsBuilder.restoreInsertionPoint(prevLoc); - - return classOp; -} - PathOp circt::firrtl::createPathRef(Operation *op, hw::HierPathOp nla, mlir::ImplicitLocOpBuilder &builderOM) { diff --git a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp index 8b346b1cbe63..c2dfe84be64e 100644 --- a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp +++ b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp @@ -91,9 +91,9 @@ struct ObjectModelIR { }; StringRef extraPortFields[3] = {"name", "direction", "width"}; - extraPortsClass = - buildSimpleClassOp(builderOM, unknownLoc, "ExtraPortsMemorySchema", - extraPortFields, extraPortsType); + extraPortsClass = ClassOp::buildSimpleClassOp( + builderOM, unknownLoc, "ExtraPortsMemorySchema", extraPortFields, + extraPortsType); mlir::Type classFieldTypes[13] = { StringType::get(context), @@ -114,8 +114,8 @@ struct ObjectModelIR { }; memorySchemaClass = - buildSimpleClassOp(builderOM, unknownLoc, "MemorySchema", - memoryParamNames, classFieldTypes); + ClassOp::buildSimpleClassOp(builderOM, unknownLoc, "MemorySchema", + memoryParamNames, classFieldTypes); // Now create the class that will instantiate metadata class with all the // memories of the circt. @@ -129,9 +129,9 @@ struct ObjectModelIR { auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( unknownLoc, circtOp.getBodyBlock()); Type classFieldTypes[] = {StringType::get(context)}; - retimeModulesSchemaClass = - buildSimpleClassOp(builderOM, unknownLoc, "RetimeModulesSchema", - retimeModulesParamNames, classFieldTypes); + retimeModulesSchemaClass = ClassOp::buildSimpleClassOp( + builderOM, unknownLoc, "RetimeModulesSchema", retimeModulesParamNames, + classFieldTypes); SmallVector mports; retimeModulesMetadataClass = builderOM.create( @@ -168,9 +168,9 @@ struct ObjectModelIR { auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( unknownLoc, circtOp.getBodyBlock()); Type classFieldTypes[] = {StringType::get(context)}; - blackBoxModulesSchemaClass = - buildSimpleClassOp(builderOM, unknownLoc, "SitestBlackBoxModulesSchema", - blackBoxModulesParamNames, classFieldTypes); + blackBoxModulesSchemaClass = ClassOp::buildSimpleClassOp( + builderOM, unknownLoc, "SitestBlackBoxModulesSchema", + blackBoxModulesParamNames, classFieldTypes); SmallVector mports; blackBoxMetadataClass = builderOM.create( builderOM.getStringAttr("SitestBlackBoxMetadata"), mports); diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index a61c106c368f..e919377f974a 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -165,6 +165,7 @@ struct ExtractInstancesPass DenseMap moduleNamespaces; /// The metadata class ops. ClassOp extractMetadataClass, schemaClass; + const unsigned prefixNameFieldId = 0, pathFieldId = 2, fileNameFieldId = 4; /// Cache of the inner ref to the new instances created. Will be used to /// create a path to the instance DenseMap innerRefToInstances; @@ -209,13 +210,8 @@ void ExtractInstancesPass::runOnOperation() { if (anyFailures) return signalPassFailure(); - ClassOp sifiveMetadata; - for (ClassOp classOp : circtOp.getOps()) { - if (classOp.getSymNameAttr().getValue().compare("SiFive_Metadata")) - continue; - sifiveMetadata = classOp; - break; - } + ClassOp sifiveMetadata = + dyn_cast_or_null(symbolTable->lookup("SiFive_Metadata")); // Generate the trace files that list where each instance was extracted from. createTraceFiles(sifiveMetadata); @@ -843,6 +839,7 @@ void ExtractInstancesPass::extractInstances() { // No update to NLATable required, since it will be deleted from the // parent, and it should already exist in the new parent module. + continue; } AnnotationSet newInstAnnos(newInst); newInstAnnos.addAnnotations(newInstNonlocalAnnos); @@ -1083,7 +1080,8 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { inst.getLoc(), extractMetadataClass.getBodyBlock()); auto prefixName = builderOM.create(prefix); auto object = builderOM.create(schemaClass, prefix); - auto fPrefix = builderOM.create(object, 0); + auto fPrefix = + builderOM.create(object, prefixNameFieldId); builderOM.create(fPrefix, prefixName); auto targetInstance = innerRefToInstances[path.front()]; @@ -1092,12 +1090,13 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { ArrayAttr::get(circtOp->getContext(), pathOpAttr)); auto pathOp = createPathRef(targetInstance, nla, builderOM); - builderOM.create( - builderOM.create(object, 2), pathOp); - builderOM.create( - builderOM.create(object, 4), - builderOM.create( - builder.getStringAttr(fileName))); + auto fPath = builderOM.create(object, pathFieldId); + builderOM.create(fPath, pathOp); + auto fFile = + builderOM.create(object, fileNameFieldId); + auto fileNameOp = + builderOM.create(builder.getStringAttr(fileName)); + builderOM.create(fFile, fileNameOp); // Now add this to the output field of the class. auto portIndex = extractMetadataClass.getNumPorts(); @@ -1182,7 +1181,7 @@ void ExtractInstancesPass::createSchema() { }; StringRef portFields[] = {"name", "path", "filename"}; - schemaClass = buildSimpleClassOp( + schemaClass = ClassOp::buildSimpleClassOp( builderOM, unknownLoc, "ExtractInstancesSchema", portFields, portsType); // Now create the class that will instantiate the schema objects. From bf41d439f51ee97e1b232d76badbe45c301da3e7 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Wed, 9 Oct 2024 07:55:10 -0700 Subject: [PATCH 5/9] Fix clang-format --- include/circt/Dialect/FIRRTL/FIRRTLUtils.h | 3 ++- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h index 3c4da7c98aad..d525850dd6bd 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h @@ -228,7 +228,8 @@ inline FIRRTLBaseType getBaseType(Type type) { } /// Get base type if isa<> the requested type, else null. -template inline T getBaseOfType(Type type) { +template +inline T getBaseOfType(Type type) { return dyn_cast_or_null(getBaseType(type)); } diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 1059c4f9cfe3..5508b63ad6be 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -4619,7 +4619,8 @@ void SubtagOp::print(::mlir::OpAsmPrinter &printer) { printer << " : " << getInput().getType(); } -template static LogicalResult verifySubfieldLike(OpTy op) { +template +static LogicalResult verifySubfieldLike(OpTy op) { if (op.getFieldIndex() >= firrtl::type_cast(op.getInput().getType()) .getNumElements()) From 568c7fcaf6a821a0c4e7a21ec4f86d11d623950f Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 10 Oct 2024 08:38:58 -0700 Subject: [PATCH 6/9] Ensure deterministic order of fields in the class --- .../FIRRTL/Transforms/ExtractInstances.cpp | 86 +++++++++++-------- test/Dialect/FIRRTL/extract-instances.mlir | 23 +++-- 2 files changed, 64 insertions(+), 45 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index e919377f974a..6e830b760267 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -1023,7 +1023,7 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { LLVM_DEBUG(llvm::dbgs() << "\nGenerating trace files\n"); // Group the extracted instances by their trace file name. - SmallDenseMap> instsByTraceFile; + llvm::MapVector> instsByTraceFile; for (auto &[inst, info] : extractedInstances) if (!info.traceFilename.empty()) instsByTraceFile[info.traceFilename].push_back(inst); @@ -1034,7 +1034,28 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { if (sifiveMetadataClass && !extractMetadataClass) createSchema(); + auto addPortsToClass = [&](ArrayRef> objFields, + ClassOp classOp) { + auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( + classOp.getLoc(), classOp.getBodyBlock()); + auto portIndex = classOp.getNumPorts(); + SmallVector> newPorts; + for (auto [index, port] : enumerate(objFields)) { + portIndex += index; + auto obj = port.first; + newPorts.emplace_back( + portIndex, + PortInfo(builderOM.getStringAttr(port.second + Twine(portIndex)), + obj.getType(), Direction::Out)); + auto blockarg = + classOp.getBodyBlock()->addArgument(obj.getType(), obj.getLoc()); + builderOM.create(blockarg, obj); + } + classOp.insertPorts(newPorts); + }; + HierPathCache pathCache(circtOp, *symbolTable); + SmallVector> classFields; for (auto &[fileName, insts] : instsByTraceFile) { LLVM_DEBUG(llvm::dbgs() << "- " << fileName << "\n"); std::string buffer; @@ -1099,14 +1120,7 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { builderOM.create(fFile, fileNameOp); // Now add this to the output field of the class. - auto portIndex = extractMetadataClass.getNumPorts(); - SmallVector> newPorts = { - {portIndex, PortInfo(builderOM.getStringAttr(prefix + "_field"), - object.getType(), Direction::Out)}}; - extractMetadataClass.insertPorts(newPorts); - auto blockarg = extractMetadataClass.getBodyBlock()->addArgument( - object.getType(), inst->getLoc()); - builderOM.create(blockarg, object); + classFields.emplace_back(object, prefix + "_field"); } // HACK: To match the Scala implementation, we strip all non-DUT modules // from the path and make the path look like it's rooted at the first DUT @@ -1132,40 +1146,36 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { // module. os << "\n"; } - if (sifiveMetadataClass) { - auto unknownLoc = UnknownLoc::get(circtOp->getContext()); - auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( - unknownLoc, sifiveMetadataClass.getBodyBlock()); - auto addPort = [&](Value obj, StringRef fieldName) { - auto portIndex = sifiveMetadataClass.getNumPorts(); - SmallVector> newPorts = { - {portIndex, PortInfo(builder.getStringAttr(fieldName + "_field_" + - Twine(portIndex)), - obj.getType(), Direction::Out)}}; - sifiveMetadataClass.insertPorts(newPorts); - auto blockarg = sifiveMetadataClass.getBodyBlock()->addArgument( - obj.getType(), unknownLoc); - builderOM.create(blockarg, obj); - }; - addPort(builderOM.create( - extractMetadataClass, - builder.getStringAttr("extract_instances_metadata")), - "extractedInstances"); - auto *node = instanceGraph->lookup(sifiveMetadataClass); - assert(node && node->hasOneUse()); - Operation *metadataObj = (*node->usesBegin())->getInstance(); - assert(isa(metadataObj)); - builderOM.setInsertionPoint(metadataObj); - auto newObj = builderOM.create( - sifiveMetadataClass, builder.getStringAttr("sifive_metadata")); - metadataObj->replaceAllUsesWith(newObj); - metadataObj->remove(); - } // Put the information in a verbatim operation. builder.create(builder.getUnknownLoc(), buffer, ValueRange{}, builder.getArrayAttr(symbols)); } + if (!classFields.empty()) { + addPortsToClass(classFields, extractMetadataClass); + // This extract instances metadata class, now needs to be instantiated + // inside the SifiveMetadata class. This also updates its signature, so keep + // the object of the SifiveMetadata class updated. + auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( + sifiveMetadataClass->getLoc(), sifiveMetadataClass.getBodyBlock()); + SmallVector> classFields = { + {builderOM.create( + extractMetadataClass, + builderOM.getStringAttr("extract_instances_metadata")), + "extractedInstances_field"}}; + + addPortsToClass(classFields, sifiveMetadataClass); + auto *node = instanceGraph->lookup(sifiveMetadataClass); + assert(node && node->hasOneUse()); + ObjectOp metadataObj = + dyn_cast_or_null((*node->usesBegin())->getInstance()); + assert(metadataObj); + builderOM.setInsertionPoint(metadataObj); + auto newObj = + builderOM.create(sifiveMetadataClass, metadataObj.getName()); + metadataObj->replaceAllUsesWith(newObj); + metadataObj->remove(); + } } void ExtractInstancesPass::createSchema() { diff --git a/test/Dialect/FIRRTL/extract-instances.mlir b/test/Dialect/FIRRTL/extract-instances.mlir index 048b5e1852e8..d84e6610bb4f 100644 --- a/test/Dialect/FIRRTL/extract-instances.mlir +++ b/test/Dialect/FIRRTL/extract-instances.mlir @@ -52,16 +52,16 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi firrtl.connect %out, %dut_out : !firrtl.uint<8>, !firrtl.uint<8> firrtl.connect %dut_in, %in : !firrtl.uint<8>, !firrtl.uint<8> %sifive_metadata = firrtl.object @SiFive_Metadata() - // CHECK: firrtl.object @SiFive_Metadata(out extractedInstances_field_0: !firrtl.class<@ExtractInstancesMetadata + // CHECK: firrtl.object @SiFive_Metadata(out [[extractedInstances_field_0:.+]]: !firrtl.class<@ExtractInstancesMetadata %0 = firrtl.object.anyref_cast %sifive_metadata : !firrtl.class<@SiFive_Metadata()> firrtl.propassign %metadataObj, %0 : !firrtl.anyref } firrtl.class @SiFive_Metadata() {} // CHECK: firrtl.class @SiFive_Metadata( - // CHECK-SAME: out %extractedInstances_field_0: !firrtl.class<@ExtractInstancesMetadata + // CHECK-SAME: out %[[extractedInstances_field_0]]: !firrtl.class<@ExtractInstancesMetadata // CHECK-SAME: { - // CHECK: %extract_instances_metadata = firrtl.object @ExtractInstancesMetadata(out bb_0_field: !firrtl.class<@ExtractInstancesSchema - // CHECK: firrtl.propassign %extractedInstances_field_0, %extract_instances_metadata : !firrtl.class<@ExtractInstancesMetadata + // CHECK: %extract_instances_metadata = firrtl.object @ExtractInstancesMetadata(out [[bb_0_field:.+]]: !firrtl.class<@ExtractInstancesSchema + // 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) { @@ -70,7 +70,7 @@ firrtl.circuit "ExtractBlackBoxesSimple" attributes {annotations = [{class = "fi // CHECK: firrtl.propassign %filename, %filename_in : !firrtl.string // CHECK: } - // CHECK: firrtl.class @ExtractInstancesMetadata(out %bb_0_field: !firrtl.class<@ExtractInstancesSchema + // CHECK: firrtl.class @ExtractInstancesMetadata(out %[[bb_0_field]]: !firrtl.class<@ExtractInstancesSchema // CHECK-SAME: { // CHECK: %[[V0:.+]] = firrtl.string "bb_0" // CHECK: %[[bb_0:.+]] = firrtl.object @ExtractInstancesSchema @@ -82,7 +82,7 @@ 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: firrtl.propassign %bb_0_field, %[[bb_0]] + // CHECK: firrtl.propassign %[[bb_0_field]], %[[bb_0]] // CHECK: } // CHECK: emit.file "BlackBoxes.txt" { @@ -433,14 +433,23 @@ firrtl.circuit "ExtractClockGatesComposed" attributes {annotations = [ firrtl.connect %child_en, %en : !firrtl.uint<1>, !firrtl.uint<1> } // CHECK-LABEL: firrtl.module @ExtractClockGatesComposed - firrtl.module @ExtractClockGatesComposed(in %clock: !firrtl.clock, in %en: !firrtl.uint<1>) { + firrtl.module @ExtractClockGatesComposed(in %clock: !firrtl.clock, in %en: !firrtl.uint<1>, out %metadataObj: !firrtl.anyref) { // CHECK: firrtl.instance gate sym [[SYM0]] @EICG_wrapper // CHECK: firrtl.instance gate sym [[SYM1]] @EICG_wrapper // CHECK: firrtl.instance mem_ext @mem_ext %dut_clock, %dut_en = firrtl.instance dut @DUTModule(in clock: !firrtl.clock, in en: !firrtl.uint<1>) firrtl.connect %dut_clock, %clock : !firrtl.clock, !firrtl.clock firrtl.connect %dut_en, %en : !firrtl.uint<1>, !firrtl.uint<1> + %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)>)>) + %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 " From 828ea9488387d1b4289d47438530b0aa52fc6518 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 10 Oct 2024 13:24:52 -0700 Subject: [PATCH 7/9] Address review comments --- .../circt/Dialect/FIRRTL/FIRRTLStructure.td | 6 +++--- include/circt/Dialect/FIRRTL/FIRRTLUtils.h | 5 ++--- .../FIRRTL/Transforms/ExtractInstances.cpp | 20 +++++++++++-------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td index 93ef916d957d..48a3ec1be8c1 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td @@ -364,9 +364,9 @@ def ClassOp : FIRRTLModuleLike<"class", [ // This builds a ClassOp, with the specified fieldNames and fieldTypes as // ports. The output property is set from the input property port. ClassOp static buildSimpleClassOp( - mlir::OpBuilder &odsBuilder, mlir::Location loc, mlir::Twine name, - mlir::ArrayRef fieldNames, - mlir::ArrayRef fieldTypes); + mlir::OpBuilder &odsBuilder, mlir::Location loc, mlir::Twine name, + mlir::ArrayRef fieldNames, + mlir::ArrayRef fieldTypes); using iterator = Block::iterator; iterator begin() { return getBodyBlock()->begin(); } diff --git a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h index d525850dd6bd..b0bc25572d04 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h @@ -228,8 +228,7 @@ inline FIRRTLBaseType getBaseType(Type type) { } /// Get base type if isa<> the requested type, else null. -template -inline T getBaseOfType(Type type) { +template inline T getBaseOfType(Type type) { return dyn_cast_or_null(getBaseType(type)); } @@ -323,7 +322,7 @@ static ResultTy transformReduce(MLIRContext *context, RangeTy &&r, void makeCommonPrefix(SmallString<64> &a, StringRef b); //===----------------------------------------------------------------------===// -// OM dialect utilities +// Object related utilities //===----------------------------------------------------------------------===// /// Add the tracker annotation to the op and get a PathOp to the op. diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 6e830b760267..54b238fdd883 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -116,7 +116,7 @@ struct ExtractInstancesPass bool anythingChanged; bool anyFailures; - CircuitOp circtOp; + CircuitOp circuitOp; InstanceGraph *instanceGraph = nullptr; SymbolTable *symbolTable = nullptr; @@ -174,7 +174,7 @@ struct ExtractInstancesPass /// Emit the annotated source code for black boxes in a circuit. void ExtractInstancesPass::runOnOperation() { - circtOp = getOperation(); + circuitOp = getOperation(); anythingChanged = false; anyFailures = false; annotatedModules.clear(); @@ -190,7 +190,10 @@ void ExtractInstancesPass::runOnOperation() { instPrefices.clear(); moduleNamespaces.clear(); circuitNamespace.clear(); - circuitNamespace.add(circtOp); + circuitNamespace.add(circuitOp); + innerRefToInstances.clear(); + extractMetadataClass = {}; + schemaClass = {}; // Walk the IR and gather all the annotations relevant for extraction that // appear on instances and the instantiated modules. @@ -1054,7 +1057,7 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { classOp.insertPorts(newPorts); }; - HierPathCache pathCache(circtOp, *symbolTable); + HierPathCache pathCache(circuitOp, *symbolTable); SmallVector> classFields; for (auto &[fileName, insts] : instsByTraceFile) { LLVM_DEBUG(llvm::dbgs() << "- " << fileName << "\n"); @@ -1108,7 +1111,7 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { auto targetInstance = innerRefToInstances[path.front()]; SmallVector pathOpAttr(llvm::reverse(path)); auto nla = pathCache.getOpFor( - ArrayAttr::get(circtOp->getContext(), pathOpAttr)); + ArrayAttr::get(circuitOp->getContext(), pathOpAttr)); auto pathOp = createPathRef(targetInstance, nla, builderOM); auto fPath = builderOM.create(object, pathFieldId); @@ -1169,7 +1172,8 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { assert(node && node->hasOneUse()); ObjectOp metadataObj = dyn_cast_or_null((*node->usesBegin())->getInstance()); - assert(metadataObj); + assert(metadataObj && + "expected the class to be instantiated by an object op"); builderOM.setInsertionPoint(metadataObj); auto newObj = builderOM.create(sifiveMetadataClass, metadataObj.getName()); @@ -1180,10 +1184,10 @@ void ExtractInstancesPass::createTraceFiles(ClassOp &sifiveMetadataClass) { void ExtractInstancesPass::createSchema() { - auto *context = circtOp->getContext(); + auto *context = circuitOp->getContext(); auto unknownLoc = mlir::UnknownLoc::get(context); auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( - unknownLoc, circtOp.getBodyBlock()); + unknownLoc, circuitOp.getBodyBlock()); mlir::Type portsType[] = { StringType::get(context), // name PathType::get(context), // extracted instance path From 8e2989585576bbff2d7eeed0d8dfe96ced49d521 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 10 Oct 2024 15:36:19 -0700 Subject: [PATCH 8/9] Move to builders --- .../circt/Dialect/FIRRTL/FIRRTLStructure.td | 11 +++++-- include/circt/Dialect/FIRRTL/FIRRTLUtils.h | 3 +- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 30 +++++++++---------- .../Transforms/CreateSiFiveMetadata.cpp | 21 ++++++------- .../FIRRTL/Transforms/ExtractInstances.cpp | 4 +-- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td index 48a3ec1be8c1..a6e31c600ba9 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td @@ -356,13 +356,18 @@ def ClassOp : FIRRTLModuleLike<"class", [ let builders = [ OpBuilder<(ins "StringAttr":$name, - "ArrayRef":$ports)>]; + "ArrayRef":$ports)>, + // This builds a ClassOp, with the specified fieldNames and fieldTypes as + // ports. The output property is set from the input property port. + OpBuilder<(ins + "Twine":$name, + "mlir::ArrayRef":$fieldNames, + "mlir::ArrayRef":$fieldTypes)> + ]; let extraModuleClassDeclaration = [{ Block *getBodyBlock() { return &getBody().front(); } - // This builds a ClassOp, with the specified fieldNames and fieldTypes as - // ports. The output property is set from the input property port. ClassOp static buildSimpleClassOp( mlir::OpBuilder &odsBuilder, mlir::Location loc, mlir::Twine name, mlir::ArrayRef fieldNames, diff --git a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h index b0bc25572d04..2def9dfc773a 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLUtils.h +++ b/include/circt/Dialect/FIRRTL/FIRRTLUtils.h @@ -228,7 +228,8 @@ inline FIRRTLBaseType getBaseType(Type type) { } /// Get base type if isa<> the requested type, else null. -template inline T getBaseOfType(Type type) { +template +inline T getBaseOfType(Type type) { return dyn_cast_or_null(getBaseType(type)); } diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 5508b63ad6be..b91b519d48d5 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -1987,13 +1987,11 @@ void ClassOp::build(OpBuilder &builder, OperationState &result, StringAttr name, body->addArgument(elt.type, elt.loc); } -void ClassOp::print(OpAsmPrinter &p) { - printClassLike(p, cast(getOperation())); -} +void ClassOp::build(::mlir::OpBuilder &odsBuilder, + ::mlir::OperationState &odsState, Twine name, + mlir::ArrayRef fieldNames, + mlir::ArrayRef fieldTypes) { -ClassOp ClassOp::buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, - Twine name, ArrayRef fieldNames, - ArrayRef fieldTypes) { SmallVector ports; for (auto [fieldName, fieldType] : llvm::zip(fieldNames, fieldTypes)) { ports.emplace_back(odsBuilder.getStringAttr(fieldName + "_in"), fieldType, @@ -2001,19 +1999,20 @@ ClassOp ClassOp::buildSimpleClassOp(OpBuilder &odsBuilder, Location loc, ports.emplace_back(odsBuilder.getStringAttr(fieldName), fieldType, Direction::Out); } - - ClassOp classOp = - odsBuilder.create(loc, odsBuilder.getStringAttr(name), ports); - Block *body = classOp.getBodyBlock(); + build(odsBuilder, odsState, odsBuilder.getStringAttr(name), ports); + // Create a region and a block for the body. + auto &body = odsState.regions[0]->getBlocks().front(); auto prevLoc = odsBuilder.saveInsertionPoint(); - odsBuilder.setInsertionPointToEnd(body); - auto args = body->getArguments(); + odsBuilder.setInsertionPointToEnd(&body); + auto args = body.getArguments(); + auto loc = odsState.location; for (unsigned i = 0, e = ports.size(); i != e; i += 2) odsBuilder.create(loc, args[i + 1], args[i]); odsBuilder.restoreInsertionPoint(prevLoc); - - return classOp; +} +void ClassOp::print(OpAsmPrinter &p) { + printClassLike(p, cast(getOperation())); } ParseResult ClassOp::parse(OpAsmParser &parser, OperationState &result) { @@ -4619,8 +4618,7 @@ void SubtagOp::print(::mlir::OpAsmPrinter &printer) { printer << " : " << getInput().getType(); } -template -static LogicalResult verifySubfieldLike(OpTy op) { +template static LogicalResult verifySubfieldLike(OpTy op) { if (op.getFieldIndex() >= firrtl::type_cast(op.getInput().getType()) .getNumElements()) diff --git a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp index c2dfe84be64e..6e11cc513c18 100644 --- a/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp +++ b/lib/Dialect/FIRRTL/Transforms/CreateSiFiveMetadata.cpp @@ -91,9 +91,8 @@ struct ObjectModelIR { }; StringRef extraPortFields[3] = {"name", "direction", "width"}; - extraPortsClass = ClassOp::buildSimpleClassOp( - builderOM, unknownLoc, "ExtraPortsMemorySchema", extraPortFields, - extraPortsType); + extraPortsClass = builderOM.create( + "ExtraPortsMemorySchema", extraPortFields, extraPortsType); mlir::Type classFieldTypes[13] = { StringType::get(context), @@ -113,9 +112,8 @@ struct ObjectModelIR { ListType::get(context, cast(StringType::get(context))), }; - memorySchemaClass = - ClassOp::buildSimpleClassOp(builderOM, unknownLoc, "MemorySchema", - memoryParamNames, classFieldTypes); + memorySchemaClass = builderOM.create( + "MemorySchema", memoryParamNames, classFieldTypes); // Now create the class that will instantiate metadata class with all the // memories of the circt. @@ -129,9 +127,8 @@ struct ObjectModelIR { auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( unknownLoc, circtOp.getBodyBlock()); Type classFieldTypes[] = {StringType::get(context)}; - retimeModulesSchemaClass = ClassOp::buildSimpleClassOp( - builderOM, unknownLoc, "RetimeModulesSchema", retimeModulesParamNames, - classFieldTypes); + retimeModulesSchemaClass = builderOM.create( + "RetimeModulesSchema", retimeModulesParamNames, classFieldTypes); SmallVector mports; retimeModulesMetadataClass = builderOM.create( @@ -168,9 +165,9 @@ struct ObjectModelIR { auto builderOM = mlir::ImplicitLocOpBuilder::atBlockEnd( unknownLoc, circtOp.getBodyBlock()); Type classFieldTypes[] = {StringType::get(context)}; - blackBoxModulesSchemaClass = ClassOp::buildSimpleClassOp( - builderOM, unknownLoc, "SitestBlackBoxModulesSchema", - blackBoxModulesParamNames, classFieldTypes); + blackBoxModulesSchemaClass = + builderOM.create("SitestBlackBoxModulesSchema", + blackBoxModulesParamNames, classFieldTypes); SmallVector mports; blackBoxMetadataClass = builderOM.create( builderOM.getStringAttr("SitestBlackBoxMetadata"), mports); diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 54b238fdd883..cc64d74da0fb 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -1195,8 +1195,8 @@ void ExtractInstancesPass::createSchema() { }; StringRef portFields[] = {"name", "path", "filename"}; - schemaClass = ClassOp::buildSimpleClassOp( - builderOM, unknownLoc, "ExtractInstancesSchema", portFields, portsType); + schemaClass = builderOM.create("ExtractInstancesSchema", portFields, + portsType); // Now create the class that will instantiate the schema objects. SmallVector mports; From d6d258793e9630c4df0b6bb024ab9228018aa2e4 Mon Sep 17 00:00:00 2001 From: Prithayan Barua Date: Thu, 10 Oct 2024 21:01:49 -0700 Subject: [PATCH 9/9] Fix clang-format --- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index b91b519d48d5..8a4d6ec7c0fe 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -4618,7 +4618,8 @@ void SubtagOp::print(::mlir::OpAsmPrinter &printer) { printer << " : " << getInput().getType(); } -template static LogicalResult verifySubfieldLike(OpTy op) { +template +static LogicalResult verifySubfieldLike(OpTy op) { if (op.getFieldIndex() >= firrtl::type_cast(op.getInput().getType()) .getNumElements())