From b2fad327ce40d86f0765a1a30a2aa05413eb0b8f Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 24 Sep 2025 18:28:44 -0400 Subject: [PATCH 1/4] [FIRRTL] Make Domains ClassLike Change domains to behave almost exactly like classes. They now have input and outputs (which can be used by their existing bodies). This could alternatively be solved by using a simpler representation for domains. However, this would be better handled with a full cleanup of how classes are modeled. Signed-off-by: Schuyler Eldridge --- .../circt/Dialect/FIRRTL/FIRRTLStatements.td | 2 +- .../circt/Dialect/FIRRTL/FIRRTLStructure.td | 74 +++++++++++---- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 90 +++++++++++++++++++ lib/Dialect/FIRRTL/Import/FIRParser.cpp | 23 +++-- .../FIRRTL/Transforms/LowerDomains.cpp | 22 ++++- test/Dialect/FIRRTL/emit-basic.mlir | 2 +- test/Dialect/FIRRTL/errors.mlir | 6 +- test/Dialect/FIRRTL/lower-domains.mlir | 32 +++++-- test/Dialect/FIRRTL/parse-basic.fir | 7 ++ test/Dialect/FIRRTL/round-trip.mlir | 9 +- 10 files changed, 229 insertions(+), 38 deletions(-) diff --git a/include/circt/Dialect/FIRRTL/FIRRTLStatements.td b/include/circt/Dialect/FIRRTL/FIRRTLStatements.td index 8b285b62d83b..fabc92a8bb09 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLStatements.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLStatements.td @@ -323,7 +323,7 @@ def MatchOp : FIRRTLOp<"match", [SingleBlock, NoTerminator, } def PropAssignOp : FIRRTLOp<"propassign", [FConnectLike, SameTypeOperands, - ParentOneOf<["FModuleOp", "ClassOp", "LayerBlockOp"]>]> { + ParentOneOf<["FModuleOp", "ClassOp", "LayerBlockOp", "DomainOp"]>]> { let summary = "Assign to a sink property value."; let description = [{ Assign an output property value. The types must match exactly. diff --git a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td index cfecd3e945fd..f66ede31c99c 100644 --- a/include/circt/Dialect/FIRRTL/FIRRTLStructure.td +++ b/include/circt/Dialect/FIRRTL/FIRRTLStructure.td @@ -327,15 +327,22 @@ def FMemModuleOp : FIRRTLModuleLike<"memmodule"> { let hasCustomAssemblyFormat = 1; } -def ClassOp : FIRRTLModuleLike<"class", [ - SingleBlock, NoTerminator, - DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods]> { +class ClassLikeOp traits = []> : + FIRRTLModuleLike, + DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods + ]> { +} + +def ClassOp : ClassLikeOp<"class"> { let summary = "FIRRTL Class"; let description = [{ The "firrtl.class" operation defines a class of property-only objects, @@ -602,23 +609,54 @@ def SimulationOp : FIRRTLOp<"simulation", [ }]; } -def DomainOp : FIRRTLOp<"domain", [ - DeclareOpInterfaceMethods, - HasParent<"firrtl::CircuitOp">, - NoTerminator, - SingleBlock -]> { +def DomainOp : ClassLikeOp<"domain"> { let summary = "Define a domain type"; let description = [{ A `firrtl.domain` operation defines a type of domain that exists in this circuit. E.g., this can be used to declare a `ClockDomain` type when modeling clocks in FIRRTL Dialect. }]; - let arguments = (ins SymbolNameAttr:$sym_name); + let arguments = ( + ins SymbolNameAttr:$sym_name, + DenseBoolArrayAttr:$portDirections, + ArrayRefAttr:$portNames, + ArrayRefAttr:$portTypes, + ArrayRefAttr:$portSymbols, + ArrayRefAttr:$portLocations, + DefaultValuedAttr:$domainInfo + ); let results = (outs); let regions = (region SizedRegion<1>:$body); - let assemblyFormat = [{ - $sym_name attr-dict-with-keyword $body + let hasCustomAssemblyFormat = 1; + let skipDefaultBuilders = 1; + let builders = [ + OpBuilder<(ins + "StringAttr":$name, + "ArrayRef":$ports)>, + ]; + + let extraModuleClassDeclaration = [{ + Block *getBodyBlock() { return &getBody().front(); } + + using iterator = Block::iterator; + iterator begin() { return getBodyBlock()->begin(); } + iterator end() { return getBodyBlock()->end(); } + + Block::BlockArgListType getArguments() { + return getBodyBlock()->getArguments(); + } + + // Return the block argument for the port with the specified index. + BlockArgument getArgument(size_t portNumber); + + OpBuilder getBodyBuilder() { + assert(!getBody().empty() && "Unexpected empty 'body' region."); + Block &bodyBlock = getBody().front(); + return OpBuilder::atBlockEnd(&bodyBlock); + } + + void getAsmBlockArgumentNames(mlir::Region ®ion, + mlir::OpAsmSetValueNameFn setNameFn); }]; } diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 7502e03b3056..1bc2f79fc9fa 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -6908,6 +6908,96 @@ LogicalResult BindOp::verifyInnerRefs(hw::InnerRefNamespace &ns) { return success(); } +//===----------------------------------------------------------------------===// +// DomainOp +//===----------------------------------------------------------------------===// + +void DomainOp::build(OpBuilder &builder, OperationState &result, + StringAttr name, ArrayRef ports) { + assert( + llvm::all_of(ports, + [](const auto &port) { return port.annotations.empty(); }) && + "domain ports may not have annotations"); + buildClass(builder, result, name, ports); + + // Create a region and a block for the body. + auto *bodyRegion = result.regions[0].get(); + Block *body = new Block(); + bodyRegion->push_back(body); + + // Add arguments to the body block. + for (auto &elt : ports) + body->addArgument(elt.type, elt.loc); +} + +SmallVector DomainOp::getPorts() { + return ::getPortImpl(cast((Operation *)*this)); +} + +void DomainOp::erasePorts(const llvm::BitVector &portIndices) { + ::erasePorts(cast((Operation *)*this), portIndices); + getBodyBlock()->eraseArguments(portIndices); +} + +void DomainOp::insertPorts(ArrayRef> ports) { + ::insertPorts(cast((Operation *)*this), ports); +} + +Convention DomainOp::getConvention() { return Convention::Internal; } + +ConventionAttr DomainOp::getConventionAttr() { + return ConventionAttr::get(getContext(), getConvention()); +} + +ArrayAttr DomainOp::getParameters() { return {}; } + +ArrayAttr DomainOp::getPortAnnotationsAttr() { + return ArrayAttr::get(getContext(), {}); +} + +ArrayRef DomainOp::getPortAnnotations() { return {}; } + +void DomainOp::setPortAnnotationsAttr(ArrayAttr annotations) { + llvm_unreachable("domains do not support annotations"); +} + +ArrayAttr DomainOp::getLayersAttr() { return ArrayAttr::get(getContext(), {}); } + +ArrayRef DomainOp::getLayers() { return {}; } + +SmallVector<::circt::hw::PortInfo> DomainOp::getPortList() { + return ::getPortListImpl(*this); +} + +::circt::hw::PortInfo DomainOp::getPort(size_t idx) { + return ::getPortImpl(*this, idx); +} + +BlockArgument DomainOp::getArgument(size_t portNumber) { + return getBodyBlock()->getArgument(portNumber); +} + +bool DomainOp::canDiscardOnUseEmpty() { return true; } + +LogicalResult +DomainOp::verifySymbolUses(::mlir::SymbolTableCollection &symbolTable) { + return verifyPortSymbolUses(cast(getOperation()), symbolTable); +} + +void DomainOp::getAsmBlockArgumentNames(mlir::Region ®ion, + mlir::OpAsmSetValueNameFn setNameFn) { + getAsmBlockArgumentNamesImpl(getOperation(), region, setNameFn); +} + +void DomainOp::print(OpAsmPrinter &p) { + printClassLike(p, cast(getOperation())); +} + +ParseResult DomainOp::parse(OpAsmParser &parser, OperationState &result) { + auto hasSSAIdentifiers = true; + return parseClassLike(parser, result, hasSSAIdentifiers); +} + //===----------------------------------------------------------------------===// // TblGen Generated Logic. //===----------------------------------------------------------------------===// diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index bdd7dbc1ad61..2486da254547 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -5745,18 +5745,31 @@ ParseResult FIRCircuitParser::parseDomain(CircuitOp circuit, unsigned indent) { consumeToken(FIRToken::kw_domain); StringAttr name; + SmallVector portList; + SmallVector portLocs; LocWithInfo info(getToken().getLoc(), this); if (parseId(name, "domain name") || parseToken(FIRToken::colon, "expected ':' after domain definition") || - info.parseOptionalInfo()) + info.parseOptionalInfo() || parsePortList(portList, portLocs, indent)) return failure(); + if (name == circuit.getName()) + return mlir::emitError(info.getLoc(), + "domain cannot be the top of a circuit"); + + for (auto &portInfo : portList) + if (!isa(portInfo.type)) + return mlir::emitError(portInfo.loc, + "ports on domains must be properties"); + auto builder = circuit.getBodyBuilder(); - DomainOp::create(builder, info.getLoc(), name) - ->getRegion(0) - .push_back(new Block()); + auto domainOp = DomainOp::create(builder, info.getLoc(), name, portList); + domainOp.setPublic(); + deferredModules.emplace_back(DeferredModuleToParse{ + domainOp, portLocs, getLexer().getCursor(), indent}); + // domainOp->getRegion(0).push_back(new Block()); - return success(); + return skipToModuleEnd(indent); } /// extclass ::= 'extclass' id ':' info? INDENT portlist DEDENT diff --git a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp index 675eea9972a5..3fbf6f96aab4 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp @@ -45,6 +45,8 @@ #include "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" #include "circt/Dialect/FIRRTL/FIRRTLOps.h" #include "circt/Support/Debug.h" +#include "circt/Support/InstanceGraphInterface.h" +#include "mlir/IR/IRMapping.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/TypeSwitch.h" #include "llvm/Support/Debug.h" @@ -493,11 +495,22 @@ LogicalResult LowerCircuit::lowerDomain(DomainOp op) { ImplicitLocOpBuilder builder(op.getLoc(), op); auto *context = op.getContext(); auto name = op.getNameAttr(); - // TODO: Update this once DomainOps have properties. - auto classIn = ClassOp::create(builder, name, {}); + + // Create the new input class. This is what the user will need to specify to + // evaluate OM. Clone the body of the domain into the class. + auto classIn = ClassOp::create(builder, name, op.getPorts()); + builder.setInsertionPointToStart(classIn.getBodyBlock()); + mlir::IRMapping mapper; + for (auto [dom, cls] : llvm::zip(op.getArguments(), classIn.getArguments())) + mapper.map(dom, cls); + for (Operation &op : op.getBody().getOps()) + builder.clone(op, mapper); + + // Create the new output class. This is what will be returned to the user. auto classInType = classIn.getInstanceType(); auto pathListType = ListType::get(context, cast(PathType::get(context))); + builder.setInsertionPointAfter(classIn); auto classOut = ClassOp::create(builder, StringAttr::get(context, Twine(name) + "_out"), {{/*name=*/constants.getDomainInfoIn(), @@ -512,12 +525,17 @@ LogicalResult LowerCircuit::lowerDomain(DomainOp op) { {/*name=*/constants.getAssociationsOut(), /*type=*/pathListType, /*dir=*/Direction::Out}}); + + // Add propassigns into the output class. builder.setInsertionPointToStart(classOut.getBodyBlock()); PropAssignOp::create(builder, classOut.getArgument(1), classOut.getArgument(0)); PropAssignOp::create(builder, classOut.getArgument(3), classOut.getArgument(2)); + + // Update the class map and instance graph while erasing the old domain op. classes.insert({name, {classIn, classOut}}); + instanceGraph.erase(instanceGraph.lookup(op)); instanceGraph.addModule(classIn); instanceGraph.addModule(classOut); op.erase(); diff --git a/test/Dialect/FIRRTL/emit-basic.mlir b/test/Dialect/FIRRTL/emit-basic.mlir index ca9c6556c353..745817c1476a 100644 --- a/test/Dialect/FIRRTL/emit-basic.mlir +++ b/test/Dialect/FIRRTL/emit-basic.mlir @@ -956,7 +956,7 @@ firrtl.circuit "Foo" { } // CHECK-LABEL: domain ClockDomain : - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: module Domains : firrtl.module @Domains( diff --git a/test/Dialect/FIRRTL/errors.mlir b/test/Dialect/FIRRTL/errors.mlir index 84a5535a5226..d2b0bf75fa39 100644 --- a/test/Dialect/FIRRTL/errors.mlir +++ b/test/Dialect/FIRRTL/errors.mlir @@ -3047,7 +3047,7 @@ firrtl.circuit "WrongDomainKind" { // ----- firrtl.circuit "DomainInfoNotArray" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // expected-error @below {{requires valid port domains}} firrtl.module @WrongDomainPortInfo( in %A: !firrtl.domain of @ClockDomain @@ -3057,7 +3057,7 @@ firrtl.circuit "DomainInfoNotArray" { // ----- firrtl.circuit "WrongDomainPortInfo" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // expected-error @below {{domain information for domain port 'A' must be a 'FlatSymbolRefAttr'}} firrtl.module @WrongDomainPortInfo( in %A: !firrtl.domain of @ClockDomain @@ -3067,7 +3067,7 @@ firrtl.circuit "WrongDomainPortInfo" { // ----- firrtl.circuit "WrongNonDomainPortInfo" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // expected-error @below {{domain information for non-domain port 'a' must be an 'ArrayAttr'}} firrtl.module @WrongNonDomainPortInfo( in %a: !firrtl.uint<1> diff --git a/test/Dialect/FIRRTL/lower-domains.mlir b/test/Dialect/FIRRTL/lower-domains.mlir index 96edb2a0538f..3dae98e55bfb 100644 --- a/test/Dialect/FIRRTL/lower-domains.mlir +++ b/test/Dialect/FIRRTL/lower-domains.mlir @@ -16,7 +16,7 @@ firrtl.circuit "Foo" { // CHECK-SAME: out %associations_out: !firrtl.list // CHECK-NEXT: firrtl.propassign %domainInfo_out, %domainInfo_in // CHECK-NEXT: firrtl.propassign %associations_out, %associations_in - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: firrtl.module @Foo( // CHECK-SAME: in %A: !firrtl.class<@ClockDomain()> // CHECK-SAME: out %A_out: !firrtl.class<@ClockDomain_out( @@ -45,14 +45,14 @@ firrtl.circuit "Foo" { // CHECK-SAME: out %domainInfo_out: !firrtl.class<@ClockDomain()> // CHECK-SAME: in %associations_in: !firrtl.list // CHECK-SAME: out %associations_out: !firrtl.list - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: firrtl.class @ResetDomain( // CHECK-LABEL: firrtl.class @ResetDomain_out( // CHECK-SAME: in %domainInfo_in: !firrtl.class<@ResetDomain()> // CHECK-SAME: out %domainInfo_out: !firrtl.class<@ResetDomain()> // CHECK-SAME: in %associations_in: !firrtl.list // CHECK-SAME: out %associations_out: !firrtl.list - firrtl.domain @ResetDomain {} + firrtl.domain @ResetDomain() {} // CHECK-LABEL: firrtl.module @Foo( // CHECK-SAME: in %A: !firrtl.class<@ClockDomain()> // CHECK-SAME: out %A_out: !firrtl.class<@ClockDomain_out( @@ -85,7 +85,7 @@ firrtl.circuit "Foo" { // intentionally NOT testing everything here due to the complexity of // maintaining this test. firrtl.circuit "Foo" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: firrtl.module @Foo // CHECK-SAME: in %A: // CHECK-SAME: out %A_out: @@ -147,7 +147,7 @@ firrtl.circuit "Foo" { // TODO: This is currently insufficient as we don't yet have domain connection // operations. firrtl.circuit "Foo" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: firrtl.module @Foo // CHECK-NOT: in %A: !firrtl.class<@ClockDomain()> // CHECK-SAME: out %A_out: !firrtl.class<@ClockDomain_out( @@ -169,7 +169,7 @@ firrtl.circuit "Foo" { // Check the behavior of the lowering of an instance. firrtl.circuit "Foo" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: firrtl.module @Bar( // CHECK-SAME: in %A: !firrtl.class<@ClockDomain()> // CHECK-SAME: out %A_out: !firrtl.class<@ClockDomain_out( @@ -202,7 +202,7 @@ firrtl.circuit "Foo" { // Check the behavior of external modules. firrtl.circuit "Foo" { - firrtl.domain @ClockDomain {} + firrtl.domain @ClockDomain() {} // CHECK-LABEL: firrtl.extmodule @Bar( // CHECK-SAME: in A: !firrtl.class<@ClockDomain()> // CHECK-SAME: out A_out: !firrtl.class<@ClockDomain_out( @@ -214,3 +214,21 @@ firrtl.circuit "Foo" { firrtl.module @Foo( ) {} } + +// ----- + +// Check that input/output properties and domain bodies are copied over. +firrtl.circuit "Foo" { + // CHECK-LABEL: firrtl.class @ClockDomain( + // CHECK-SAME: in %name_in: !firrtl.string, + // CHECK-SAME: out %name_out: !firrtl.string + // CHECK-SAME: ) + // CHECK-NEXT: firrtl.propassign %name_out, %name_in : !firrtl.string + firrtl.domain @ClockDomain( + in %name_in: !firrtl.string, + out %name_out: !firrtl.string + ) { + firrtl.propassign %name_out, %name_in : !firrtl.string + } + firrtl.module @Foo() {} +} diff --git a/test/Dialect/FIRRTL/parse-basic.fir b/test/Dialect/FIRRTL/parse-basic.fir index a00fb0df19aa..5d3df8bf9033 100644 --- a/test/Dialect/FIRRTL/parse-basic.fir +++ b/test/Dialect/FIRRTL/parse-basic.fir @@ -2196,7 +2196,14 @@ circuit NullaryCat: FIRRTL version 5.1.0 circuit Domains: ; CHECK-LABEL: firrtl.domain @ClockDomain + ; CHECK-SAME: in %name_in: !firrtl.string + ; CHECK-SAME: out %name_out: !firrtl.string domain ClockDomain: + input name_in: String + output name_out: String + + ; CHECK-NEXT: firrtl.propassign %name_out, %name_in + propassign name_out, name_in module Foo: input A: Domain of ClockDomain diff --git a/test/Dialect/FIRRTL/round-trip.mlir b/test/Dialect/FIRRTL/round-trip.mlir index 66b6957b9970..022bebc470ef 100644 --- a/test/Dialect/FIRRTL/round-trip.mlir +++ b/test/Dialect/FIRRTL/round-trip.mlir @@ -197,7 +197,14 @@ firrtl.module @Fprintf( } // CHECK-LABEL: firrtl.domain @ClockDomain -firrtl.domain @ClockDomain { +// CHECK-SAME: in %name_in: !firrtl.string, +// CHECK-SAME: out %name_out: !firrtl.string +firrtl.domain @ClockDomain( + in %name_in: !firrtl.string, + out %name_out: !firrtl.string +) { + // CHECK-NEXT: firrtl.propassign %name_out, %name_in : !firrtl.string + firrtl.propassign %name_out, %name_in : !firrtl.string } firrtl.module @DomainsSubmodule( From 748bd39794f430384e323cd76a33c52c8b2e2d5e Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Mon, 6 Oct 2025 14:48:52 -0400 Subject: [PATCH 2/4] fixup! [FIRRTL] Make Domains ClassLike --- lib/Dialect/FIRRTL/Import/FIRParser.cpp | 1 - lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Dialect/FIRRTL/Import/FIRParser.cpp b/lib/Dialect/FIRRTL/Import/FIRParser.cpp index 2486da254547..223900b4c320 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -5767,7 +5767,6 @@ ParseResult FIRCircuitParser::parseDomain(CircuitOp circuit, unsigned indent) { domainOp.setPublic(); deferredModules.emplace_back(DeferredModuleToParse{ domainOp, portLocs, getLexer().getCursor(), indent}); - // domainOp->getRegion(0).push_back(new Block()); return skipToModuleEnd(indent); } diff --git a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp index 3fbf6f96aab4..1f20712b7cce 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp @@ -501,7 +501,8 @@ LogicalResult LowerCircuit::lowerDomain(DomainOp op) { auto classIn = ClassOp::create(builder, name, op.getPorts()); builder.setInsertionPointToStart(classIn.getBodyBlock()); mlir::IRMapping mapper; - for (auto [dom, cls] : llvm::zip(op.getArguments(), classIn.getArguments())) + for (auto [dom, cls] : + llvm::zip_equal(op.getArguments(), classIn.getArguments())) mapper.map(dom, cls); for (Operation &op : op.getBody().getOps()) builder.clone(op, mapper); From 73f465ceaf75f6d266968c9ae429c7af52810d81 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 7 Oct 2025 18:25:28 -0400 Subject: [PATCH 3/4] fixup! [FIRRTL] Make Domains ClassLike --- lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp index 1f20712b7cce..78e800130388 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp @@ -46,7 +46,6 @@ #include "circt/Dialect/FIRRTL/FIRRTLOps.h" #include "circt/Support/Debug.h" #include "circt/Support/InstanceGraphInterface.h" -#include "mlir/IR/IRMapping.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/TypeSwitch.h" #include "llvm/Support/Debug.h" @@ -499,13 +498,7 @@ LogicalResult LowerCircuit::lowerDomain(DomainOp op) { // Create the new input class. This is what the user will need to specify to // evaluate OM. Clone the body of the domain into the class. auto classIn = ClassOp::create(builder, name, op.getPorts()); - builder.setInsertionPointToStart(classIn.getBodyBlock()); - mlir::IRMapping mapper; - for (auto [dom, cls] : - llvm::zip_equal(op.getArguments(), classIn.getArguments())) - mapper.map(dom, cls); - for (Operation &op : op.getBody().getOps()) - builder.clone(op, mapper); + classIn.getBody().takeBody(op.getBody()); // Create the new output class. This is what will be returned to the user. auto classInType = classIn.getInstanceType(); From 28fca26925ece83293d55b69b42537e1efb85327 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Fri, 10 Oct 2025 16:20:41 -0400 Subject: [PATCH 4/4] fixup! [FIRRTL] Make Domains ClassLike --- lib/Dialect/FIRRTL/FIRRTLOps.cpp | 8 +++++--- test/Dialect/FIRRTL/errors.mlir | 10 ++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/Dialect/FIRRTL/FIRRTLOps.cpp b/lib/Dialect/FIRRTL/FIRRTLOps.cpp index 1bc2f79fc9fa..2533a299978d 100644 --- a/lib/Dialect/FIRRTL/FIRRTLOps.cpp +++ b/lib/Dialect/FIRRTL/FIRRTLOps.cpp @@ -3831,12 +3831,14 @@ LogicalResult ObjectOp::verifySymbolUses(SymbolTableCollection &symbolTable) { auto className = classType.getNameAttr(); // verify that the class exists. - auto classOp = dyn_cast_or_null( - symbolTable.lookupSymbolIn(circuitOp, className)); - if (!classOp) + Operation *op = symbolTable.lookupSymbolIn(circuitOp, className); + if (!op) return emitOpError() << "references unknown class " << className; + if (!isa(op)) + return emitOpError() << "cannot instantiate non-class op " << className; // verify that the result type agrees with the class definition. + auto classOp = dyn_cast(op); if (failed(classOp.verifyType(classType, [&]() { return emitOpError(); }))) return failure(); diff --git a/test/Dialect/FIRRTL/errors.mlir b/test/Dialect/FIRRTL/errors.mlir index d2b0bf75fa39..7206890c0fae 100644 --- a/test/Dialect/FIRRTL/errors.mlir +++ b/test/Dialect/FIRRTL/errors.mlir @@ -3073,3 +3073,13 @@ firrtl.circuit "WrongNonDomainPortInfo" { in %a: !firrtl.uint<1> ) attributes {domainInfo = [["hello"]]} {} } + +// ----- + +firrtl.circuit "DomainsCannotBeInstantiated" { + firrtl.domain @A() {} + firrtl.module @DomainsCannotBeInstantiated() { + // expected-error @below {{cannot instantiate non-class op @A}} + firrtl.object @A() + } +}