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..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(); @@ -6908,6 +6910,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..223900b4c320 100644 --- a/lib/Dialect/FIRRTL/Import/FIRParser.cpp +++ b/lib/Dialect/FIRRTL/Import/FIRParser.cpp @@ -5745,18 +5745,30 @@ 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}); - 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..78e800130388 100644 --- a/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp +++ b/lib/Dialect/FIRRTL/Transforms/LowerDomains.cpp @@ -45,6 +45,7 @@ #include "circt/Dialect/FIRRTL/FIRRTLInstanceGraph.h" #include "circt/Dialect/FIRRTL/FIRRTLOps.h" #include "circt/Support/Debug.h" +#include "circt/Support/InstanceGraphInterface.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/TypeSwitch.h" #include "llvm/Support/Debug.h" @@ -493,11 +494,17 @@ 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()); + classIn.getBody().takeBody(op.getBody()); + + // 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 +519,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..7206890c0fae 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,9 +3067,19 @@ 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> ) attributes {domainInfo = [["hello"]]} {} } + +// ----- + +firrtl.circuit "DomainsCannotBeInstantiated" { + firrtl.domain @A() {} + firrtl.module @DomainsCannotBeInstantiated() { + // expected-error @below {{cannot instantiate non-class op @A}} + firrtl.object @A() + } +} 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(