From e2f927bf332f8e6d4f45343bc7d2e5395837dbcc Mon Sep 17 00:00:00 2001 From: Robert Young Date: Mon, 17 Mar 2025 16:48:18 -0400 Subject: [PATCH] [FIRRTL] Add doNotPrint flag to InstanceOp This flag acts the same as doNotPrint in hardware. When lowering an instance to the hw dialect, copy the doNotPrint attribute over. --- .../Dialect/FIRRTL/FIRRTLDeclarations.td | 96 +++++++++---------- lib/Analysis/FIRRTLInstanceInfo.cpp | 11 ++- lib/Conversion/FIRRTLToHW/LowerToHW.cpp | 11 +-- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 49 ++++++---- lib/Dialect/FIRRTL/Import/FIRParser.cpp | 2 +- .../FIRRTL/Transforms/ExtractInstances.cpp | 2 +- .../FIRRTL/Transforms/InjectDUTHierarchy.cpp | 2 +- lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp | 1 + lib/Dialect/FIRRTL/Transforms/LowerMemory.cpp | 2 +- .../FIRRTL/Transforms/LowerSignatures.cpp | 2 +- lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp | 2 +- .../FIRRTL/Transforms/SpecializeOption.cpp | 2 +- test/Conversion/FIRRTLToHW/lower-to-hw.mlir | 5 + 13 files changed, 101 insertions(+), 86 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td b/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td index 2aeaca3c9dc2..a44910f1a4e0 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLDeclarations.td @@ -64,61 +64,57 @@ def InstanceOp : HardwareDeclOp<"instance", [ ``` }]; - let arguments = (ins FlatSymbolRefAttr:$moduleName, StrAttr:$name, NameKindAttr:$nameKind, - DenseBoolArrayAttr:$portDirections, StrArrayAttr:$portNames, - AnnotationArrayAttr:$annotations, - PortAnnotationsAttr:$portAnnotations, - LayerArrayAttr:$layers, - UnitAttr:$lowerToBind, - OptionalAttr:$inner_sym); + let arguments = (ins FlatSymbolRefAttr:$moduleName, StrAttr:$name, + NameKindAttr:$nameKind, DenseBoolArrayAttr:$portDirections, + StrArrayAttr:$portNames, AnnotationArrayAttr:$annotations, + PortAnnotationsAttr:$portAnnotations, LayerArrayAttr:$layers, + UnitAttr:$lowerToBind, UnitAttr:$doNotPrint, + OptionalAttr:$inner_sym); let results = (outs Variadic:$results); let hasCustomAssemblyFormat = 1; - let builders = [ - OpBuilder<(ins "::mlir::TypeRange":$resultTypes, - "::mlir::StringRef":$moduleName, - "::mlir::StringRef":$name, - "::circt::firrtl::NameKindEnum":$nameKind, - "::mlir::ArrayRef":$portDirections, - "::mlir::ArrayRef":$portNames, - CArg<"ArrayRef", "{}">:$annotations, - CArg<"ArrayRef", "{}">:$portAnnotations, - CArg<"::mlir::ArrayRef", "{}">:$layers, - CArg<"bool","false">:$lowerToBind, - CArg<"StringAttr", "StringAttr()">:$innerSym)>, - OpBuilder<(ins "::mlir::TypeRange":$resultTypes, - "::mlir::StringRef":$moduleName, - "::mlir::StringRef":$name, - "::circt::firrtl::NameKindEnum":$nameKind, - "::mlir::ArrayRef":$portDirections, - "::mlir::ArrayRef":$portNames, - "ArrayRef":$annotations, - "ArrayRef":$portAnnotations, - "::mlir::ArrayRef":$layers, - "bool":$lowerToBind, - "hw::InnerSymAttr":$innerSym)>, - - /// Constructor when you have the target module in hand. - OpBuilder<(ins "FModuleLike":$module, - "mlir::StringRef":$name, - CArg<"NameKindEnum", "NameKindEnum::DroppableName">:$nameKind, - CArg<"ArrayRef", "{}">:$annotations, - CArg<"ArrayRef", "{}">:$portAnnotations, - CArg<"bool","false">:$lowerToBind, - CArg<"hw::InnerSymAttr", "hw::InnerSymAttr()">:$innerSym)>, - - /// Constructor when you have a port info list in hand. - OpBuilder<(ins "ArrayRef":$ports, - "::mlir::StringRef":$moduleName, - "mlir::StringRef":$name, - CArg<"NameKindEnum", "NameKindEnum::DroppableName">:$nameKind, - CArg<"ArrayRef", "{}">:$annotations, - CArg<"ArrayRef", "{}">:$layers, - CArg<"bool","false">:$lowerToBind, - CArg<"hw::InnerSymAttr", "hw::InnerSymAttr()">:$innerSym)> - ]; + let builders = + [OpBuilder<(ins "::mlir::TypeRange":$resultTypes, + "::mlir::StringRef":$moduleName, "::mlir::StringRef":$name, + "::circt::firrtl::NameKindEnum":$nameKind, + "::mlir::ArrayRef":$portDirections, + "::mlir::ArrayRef":$portNames, + CArg<"ArrayRef", "{}">:$annotations, + CArg<"ArrayRef", "{}">:$portAnnotations, + CArg<"::mlir::ArrayRef", "{}">:$layers, + CArg<"bool", "false">:$lowerToBind, + CArg<"bool", "false">:$doNotPrint, + CArg<"StringAttr", "StringAttr()">:$innerSym)>, + OpBuilder<(ins "::mlir::TypeRange":$resultTypes, + "::mlir::StringRef":$moduleName, "::mlir::StringRef":$name, + "::circt::firrtl::NameKindEnum":$nameKind, + "::mlir::ArrayRef":$portDirections, + "::mlir::ArrayRef":$portNames, + "ArrayRef":$annotations, + "ArrayRef":$portAnnotations, + "::mlir::ArrayRef":$layers, "bool":$lowerToBind, + "bool":$doNotPrint, "hw::InnerSymAttr":$innerSym)>, + + /// Constructor when you have the target module in hand. + OpBuilder<(ins "FModuleLike":$module, "mlir::StringRef":$name, + CArg<"NameKindEnum", "NameKindEnum::DroppableName">:$nameKind, + CArg<"ArrayRef", "{}">:$annotations, + CArg<"ArrayRef", "{}">:$portAnnotations, + CArg<"bool", "false">:$lowerToBind, + CArg<"bool", "false">:$doNotPrint, + CArg<"hw::InnerSymAttr", "hw::InnerSymAttr()">:$innerSym)>, + + /// Constructor when you have a port info list in hand. + OpBuilder<(ins "ArrayRef":$ports, + "::mlir::StringRef":$moduleName, "mlir::StringRef":$name, + CArg<"NameKindEnum", "NameKindEnum::DroppableName">:$nameKind, + CArg<"ArrayRef", "{}">:$annotations, + CArg<"ArrayRef", "{}">:$layers, + CArg<"bool", "false">:$lowerToBind, + CArg<"bool", "false">:$doNotPrint, + CArg<"hw::InnerSymAttr", "hw::InnerSymAttr()">:$innerSym)>]; let extraClassDeclaration = [{ /// Return the port direction for the specified result number. diff --git a/lib/Analysis/FIRRTLInstanceInfo.cpp b/lib/Analysis/FIRRTLInstanceInfo.cpp index 3e1f4b33f7af..f00782e17f98 100644 --- a/lib/Analysis/FIRRTLInstanceInfo.cpp +++ b/lib/Analysis/FIRRTLInstanceInfo.cpp @@ -140,10 +140,13 @@ InstanceInfo::InstanceInfo(Operation *op, mlir::AnalysisManager &am) { attributes.underDut.mergeIn(parentAttrs.underDut); // Update underLayer. - auto instanceOp = useIt->getInstance(); - bool underLayer = (isa(instanceOp) && - cast(instanceOp).getLowerToBind()) || - instanceOp->getParentOfType(); + bool underLayer = false; + if (auto instanceOp = useIt->getInstance()) { + if (instanceOp.getLowerToBind() || instanceOp.getDoNotPrint() || + instanceOp->getParentOfType()) + underLayer = true; + } + if (!isGCCompanion) { if (underLayer) attributes.underLayer.mergeIn(true); diff --git a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp index 4a1d8425527d..52ef6626453e 100644 --- a/lib/Conversion/FIRRTLToHW/LowerToHW.cpp +++ b/lib/Conversion/FIRRTLToHW/LowerToHW.cpp @@ -249,14 +249,13 @@ struct CircuitLoweringState { // Pre-populate the dutModules member with a list of all modules that are // determined to be under the DUT. auto inDUT = [&](igraph::ModuleOpInterface child) { - auto isBind = [](igraph::InstanceRecord *instRec) { - auto inst = instRec->getInstance(); - if (auto *finst = dyn_cast(&inst)) - return finst->getLowerToBind(); + auto isPhony = [](igraph::InstanceRecord *instRec) { + if (auto inst = instRec->getInstance()) + return inst.getLowerToBind() || inst.getDoNotPrint(); return false; }; if (auto parent = dyn_cast(*dut)) - return getInstanceGraph().isAncestor(child, parent, isBind); + return getInstanceGraph().isAncestor(child, parent, isPhony); return dut == child; }; circuitOp->walk([&](FModuleLike moduleOp) { @@ -3397,7 +3396,7 @@ LogicalResult FIRRTLLowering::visitDecl(InstanceOp oldInstance) { auto newInstance = builder.create( newModule, oldInstance.getNameAttr(), operands, parameters, innerSym); - if (oldInstance.getLowerToBind()) + if (oldInstance.getLowerToBind() || oldInstance.getDoNotPrint()) newInstance.setDoNotPrintAttr(builder.getUnitAttr()); if (newInstance.getInnerSymAttr()) diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 3f585c1c9f0d..825531d0b79b 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -2217,24 +2217,30 @@ LogicalResult LayerOp::verify() { // InstanceOp //===----------------------------------------------------------------------===// -void InstanceOp::build( - OpBuilder &builder, OperationState &result, TypeRange resultTypes, - StringRef moduleName, StringRef name, NameKindEnum nameKind, - ArrayRef portDirections, ArrayRef portNames, - ArrayRef annotations, ArrayRef portAnnotations, - ArrayRef layers, bool lowerToBind, StringAttr innerSym) { +void InstanceOp::build(OpBuilder &builder, OperationState &result, + TypeRange resultTypes, StringRef moduleName, + StringRef name, NameKindEnum nameKind, + ArrayRef portDirections, + ArrayRef portNames, + ArrayRef annotations, + ArrayRef portAnnotations, + ArrayRef layers, bool lowerToBind, + bool doNotPrint, StringAttr innerSym) { build(builder, result, resultTypes, moduleName, name, nameKind, portDirections, portNames, annotations, portAnnotations, layers, - lowerToBind, + lowerToBind, doNotPrint, innerSym ? hw::InnerSymAttr::get(innerSym) : hw::InnerSymAttr()); } -void InstanceOp::build( - OpBuilder &builder, OperationState &result, TypeRange resultTypes, - StringRef moduleName, StringRef name, NameKindEnum nameKind, - ArrayRef portDirections, ArrayRef portNames, - ArrayRef annotations, ArrayRef portAnnotations, - ArrayRef layers, bool lowerToBind, hw::InnerSymAttr innerSym) { +void InstanceOp::build(OpBuilder &builder, OperationState &result, + TypeRange resultTypes, StringRef moduleName, + StringRef name, NameKindEnum nameKind, + ArrayRef portDirections, + ArrayRef portNames, + ArrayRef annotations, + ArrayRef portAnnotations, + ArrayRef layers, bool lowerToBind, + bool doNotPrint, hw::InnerSymAttr innerSym) { result.addTypes(resultTypes); result.getOrAddProperties().setModuleName( SymbolRefAttr::get(builder.getContext(), moduleName)); @@ -2250,6 +2256,9 @@ void InstanceOp::build( if (lowerToBind) result.getOrAddProperties().setLowerToBind( builder.getUnitAttr()); + if (doNotPrint) + result.getOrAddProperties().setDoNotPrint( + builder.getUnitAttr()); if (innerSym) result.getOrAddProperties().setInnerSym(innerSym); @@ -2272,7 +2281,7 @@ void InstanceOp::build(OpBuilder &builder, OperationState &result, FModuleLike module, StringRef name, NameKindEnum nameKind, ArrayRef annotations, ArrayRef portAnnotations, bool lowerToBind, - hw::InnerSymAttr innerSym) { + bool doNotPrint, hw::InnerSymAttr innerSym) { // Gather the result types. SmallVector resultTypes; @@ -2298,7 +2307,7 @@ void InstanceOp::build(OpBuilder &builder, OperationState &result, module.getPortDirectionsAttr(), module.getPortNamesAttr(), builder.getArrayAttr(annotations), portAnnotationsAttr, module.getLayersAttr(), lowerToBind ? builder.getUnitAttr() : UnitAttr(), - innerSym); + doNotPrint ? builder.getUnitAttr() : UnitAttr(), innerSym); } void InstanceOp::build(OpBuilder &builder, OperationState &odsState, @@ -2306,7 +2315,7 @@ void InstanceOp::build(OpBuilder &builder, OperationState &odsState, StringRef name, NameKindEnum nameKind, ArrayRef annotations, ArrayRef layers, bool lowerToBind, - hw::InnerSymAttr innerSym) { + bool doNotPrint, hw::InnerSymAttr innerSym) { // Gather the result types. SmallVector newResultTypes; SmallVector newPortDirections; @@ -2321,7 +2330,7 @@ void InstanceOp::build(OpBuilder &builder, OperationState &odsState, return build(builder, odsState, newResultTypes, moduleName, name, nameKind, newPortDirections, newPortNames, annotations, newPortAnnotations, - layers, lowerToBind, innerSym); + layers, lowerToBind, doNotPrint, innerSym); } LogicalResult InstanceOp::verify() { @@ -2365,7 +2374,8 @@ InstanceOp InstanceOp::erasePorts(OpBuilder &builder, auto newOp = builder.create( getLoc(), newResultTypes, getModuleName(), getName(), getNameKind(), newPortDirections, newPortNames, getAnnotations().getValue(), - newPortAnnotations, getLayers(), getLowerToBind(), getInnerSymAttr()); + newPortAnnotations, getLayers(), getLowerToBind(), getDoNotPrint(), + getInnerSymAttr()); for (unsigned oldIdx = 0, newIdx = 0, numOldPorts = getNumResults(); oldIdx != numOldPorts; ++oldIdx) { @@ -2438,7 +2448,8 @@ InstanceOp::cloneAndInsertPorts(ArrayRef> ports) { return OpBuilder(*this).create( getLoc(), newPortTypes, getModuleName(), getName(), getNameKind(), newPortDirections, newPortNames, getAnnotations().getValue(), - newPortAnnos, getLayers(), getLowerToBind(), getInnerSymAttr()); + newPortAnnos, getLayers(), getLowerToBind(), getDoNotPrint(), + getInnerSymAttr()); } LogicalResult InstanceOp::verifySymbolUses(SymbolTableCollection &symbolTable) { diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index 016f3a74541a..cae141de2c16 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -4132,7 +4132,7 @@ ParseResult FIRStmtParser::parseInstance() { hw::InnerSymAttr sym = {}; auto result = builder.create( referencedModule, id, NameKindEnum::InterestingName, - annotations.getValue(), portAnnotations, false, sym); + annotations.getValue(), portAnnotations, false, false, sym); // Since we are implicitly unbundling the instance results, we need to keep // track of the mapping from bundle fields to results in the unbundledValues diff --git a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp index 2448610576a5..8e6c16ef452c 100644 --- a/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp +++ b/lib/Dialect/FIRRTL/Transforms/ExtractInstances.cpp @@ -969,7 +969,7 @@ void ExtractInstancesPass::groupInstances() { wrapper.getLoc(), wrapper, wrapperName, NameKindEnum::DroppableName, ArrayRef{}, /*portAnnotations=*/ArrayRef{}, /*lowerToBind=*/false, - hw::InnerSymAttr::get(wrapperInstName)); + /*doNotPrint=*/false, hw::InnerSymAttr::get(wrapperInstName)); unsigned portIdx = 0; for (auto inst : insts) for (auto result : inst.getResults()) diff --git a/lib/Dialect/FIRRTL/Transforms/InjectDUTHierarchy.cpp b/lib/Dialect/FIRRTL/Transforms/InjectDUTHierarchy.cpp index a8ac5786eca9..fcc99de28d11 100644 --- a/lib/Dialect/FIRRTL/Transforms/InjectDUTHierarchy.cpp +++ b/lib/Dialect/FIRRTL/Transforms/InjectDUTHierarchy.cpp @@ -177,7 +177,7 @@ void InjectDUTHierarchy::runOnOperation() { auto wrapperInst = b.create(b.getUnknownLoc(), wrapper, wrapper.getModuleName(), NameKindEnum::DroppableName, ArrayRef{}, - ArrayRef{}, false, + ArrayRef{}, false, false, hw::InnerSymAttr::get(b.getStringAttr( dutNS.newName(wrapper.getModuleName())))); for (const auto &pair : llvm::enumerate(wrapperInst.getResults())) { diff --git a/lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp b/lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp index 0c02f03dcd98..de54aeff8287 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerLayers.cpp @@ -652,6 +652,7 @@ LogicalResult LowerLayersPass::runOnModuleBody(FModuleOp moduleOp, instanceName, NameKindEnum::DroppableName, /*annotations=*/ArrayRef{}, /*portAnnotations=*/ArrayRef{}, /*lowerToBind=*/true, + /*doNotPrint=*/false, /*innerSym=*/ (innerSyms.empty() ? hw::InnerSymAttr{} : hw::InnerSymAttr::get(builder.getStringAttr( diff --git a/lib/Dialect/FIRRTL/Transforms/LowerMemory.cpp b/lib/Dialect/FIRRTL/Transforms/LowerMemory.cpp index 7003607ed17a..f04403247a43 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerMemory.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerMemory.cpp @@ -497,7 +497,7 @@ InstanceOp LowerMemoryPass::emitMemoryInstance(MemOp op, FModuleOp module, /*annotations=*/ArrayRef(), /*portAnnotations=*/ArrayRef(), /*layers=*/ArrayRef(), /*lowerToBind=*/false, - op.getInnerSymAttr()); + /*doNotPrint=*/false, op.getInnerSymAttr()); // Update all users of the result of read ports for (auto [subfield, result] : returnHolder) { diff --git a/lib/Dialect/FIRRTL/Transforms/LowerSignatures.cpp b/lib/Dialect/FIRRTL/Transforms/LowerSignatures.cpp index afeee9cb626c..3c2f7b48a114 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerSignatures.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerSignatures.cpp @@ -404,7 +404,7 @@ static void lowerModuleBody(FModuleOp mod, auto newOp = theBuilder.create( instPorts, inst.getModuleName(), inst.getName(), inst.getNameKind(), annos.getValue(), inst.getLayers(), inst.getLowerToBind(), - inst.getInnerSymAttr()); + inst.getDoNotPrint(), inst.getInnerSymAttr()); auto oldDict = inst->getDiscardableAttrDictionary(); auto newDict = newOp->getDiscardableAttrDictionary(); diff --git a/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp b/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp index 72a5a4c03318..96e228ae3456 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerTypes.cpp @@ -1508,7 +1508,7 @@ bool TypeLoweringVisitor::visitDecl(InstanceOp op) { op.getNameKindAttr(), direction::packAttribute(context, newDirs), builder->getArrayAttr(newNames), op.getAnnotations(), builder->getArrayAttr(newPortAnno), op.getLayersAttr(), - op.getLowerToBindAttr(), + op.getLowerToBindAttr(), op.getDoNotPrintAttr(), sym ? hw::InnerSymAttr::get(sym) : hw::InnerSymAttr()); newInstance->setDiscardableAttrs(op->getDiscardableAttrDictionary()); diff --git a/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp b/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp index c52dad732060..dcf9278b3d11 100644 --- a/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp +++ b/lib/Dialect/FIRRTL/Transforms/SpecializeOption.cpp @@ -85,7 +85,7 @@ struct SpecializeOptionPass inst.getNameKindAttr(), inst.getPortDirectionsAttr(), inst.getPortNamesAttr(), inst.getAnnotationsAttr(), inst.getPortAnnotationsAttr(), builder.getArrayAttr({}), - UnitAttr{}, inst.getInnerSymAttr()); + UnitAttr{}, UnitAttr{}, inst.getInnerSymAttr()); inst.replaceAllUsesWith(newInst); inst.erase(); diff --git a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir index d2857f86c306..01fb535da015 100644 --- a/test/Conversion/FIRRTLToHW/lower-to-hw.mlir +++ b/test/Conversion/FIRRTLToHW/lower-to-hw.mlir @@ -650,6 +650,11 @@ firrtl.circuit "Simple" attributes {annotations = [{class = firrtl.connect %qux, %dummy : !firrtl.uint<1>, !firrtl.uint<1> } + // CHECK-LABEL: hw.module @DoNotPrintTest() + firrtl.module @DoNotPrintTest() { + // CHECK: hw.instance "foo" @foo() -> () {doNotPrint} + firrtl.instance foo {doNotPrint} @foo() + } // CHECK-LABEL: hw.module private @attributes_preservation // CHECK-SAME: firrtl.foo = "bar"