[FIRRTL] Make Domains ClassLike

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.

include/circt/Dialect/FIRRTL/FIRRTLStructure.td

@@ -327,15 +327,22 @@ def FMemModuleOp : FIRRTLModuleLike<"memmodule"> {
   let hasCustomAssemblyFormat = 1;
 }
 
-def ClassOp : FIRRTLModuleLike<"class", [
-  SingleBlock, NoTerminator,
-  DeclareOpInterfaceMethods<FModuleLike, [
-    // Class Ops do not support port annotations or enabled layers.
-    // Override these methods to return an empty array attr.
-    "getPortAnnotationsAttr",
-    "getLayersAttr"]>,
-  DeclareOpInterfaceMethods<ClassLike>,
-  DeclareOpInterfaceMethods<Symbol, ["canDiscardOnUseEmpty"]>]> {
+class ClassLikeOp<string mnemonic, list<Trait> traits = []> :
+  FIRRTLModuleLike<mnemonic, traits # [
+    SingleBlock,
+    NoTerminator,
+    DeclareOpInterfaceMethods<FModuleLike, [
+      // ClassLikeOps do not support port annotations or enabled layers.
+      // Override these methods to return an empty array attr.
+      "getPortAnnotationsAttr",
+      "getLayersAttr"
+    ]>,
+    DeclareOpInterfaceMethods<ClassLike>,
+    DeclareOpInterfaceMethods<Symbol, ["canDiscardOnUseEmpty"]>
+  ]> {
+}
+
+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<Symbol>,
-  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<ArrayRefAttr, "{}">:$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<PortInfo>":$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 &region,
+                                  mlir::OpAsmSetValueNameFn setNameFn);
   }];
 }
 

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<ClassLike>(
-      symbolTable.lookupSymbolIn(circuitOp, className));
-  if (!classOp)
+  Operation *op = symbolTable.lookupSymbolIn(circuitOp, className);
+  if (!op)
     return emitOpError() << "references unknown class " << className;
+  if (!isa<ClassOp, ExtClassOp>(op))
+    return emitOpError() << "cannot instantiate non-class op " << className;
 
   // verify that the result type agrees with the class definition.
+  auto classOp = dyn_cast<ClassLike>(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<PortInfo> ports) {
+  assert(
+      llvm::all_of(ports,
+                   [](const auto &port) { return port.annotations.empty(); }) &&
+      "domain ports may not have annotations");
+  buildClass<DomainOp>(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<PortInfo> DomainOp::getPorts() {
+  return ::getPortImpl(cast<FModuleLike>((Operation *)*this));
+}
+
+void DomainOp::erasePorts(const llvm::BitVector &portIndices) {
+  ::erasePorts(cast<FModuleLike>((Operation *)*this), portIndices);
+  getBodyBlock()->eraseArguments(portIndices);
+}
+
+void DomainOp::insertPorts(ArrayRef<std::pair<unsigned, PortInfo>> ports) {
+  ::insertPorts(cast<FModuleLike>((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<Attribute> DomainOp::getPortAnnotations() { return {}; }
+
+void DomainOp::setPortAnnotationsAttr(ArrayAttr annotations) {
+  llvm_unreachable("domains do not support annotations");
+}
+
+ArrayAttr DomainOp::getLayersAttr() { return ArrayAttr::get(getContext(), {}); }
+
+ArrayRef<Attribute> 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<FModuleLike>(getOperation()), symbolTable);
+}
+
+void DomainOp::getAsmBlockArgumentNames(mlir::Region &region,
+                                        mlir::OpAsmSetValueNameFn setNameFn) {
+  getAsmBlockArgumentNamesImpl(getOperation(), region, setNameFn);
+}
+
+void DomainOp::print(OpAsmPrinter &p) {
+  printClassLike(p, cast<ClassLike>(getOperation()));
+}
+
+ParseResult DomainOp::parse(OpAsmParser &parser, OperationState &result) {
+  auto hasSSAIdentifiers = true;
+  return parseClassLike<DomainOp>(parser, result, hasSSAIdentifiers);
+}
+
 //===----------------------------------------------------------------------===//
 // TblGen Generated Logic.
 //===----------------------------------------------------------------------===//

lib/Dialect/FIRRTL/Import/FIRParser.cpp

@@ -5745,18 +5745,30 @@ ParseResult FIRCircuitParser::parseDomain(CircuitOp circuit, unsigned indent) {
   consumeToken(FIRToken::kw_domain);
 
   StringAttr name;
+  SmallVector<PortInfo, 8> portList;
+  SmallVector<SMLoc> 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<PropertyType>(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

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<PropertyType>(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();

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(

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<IntegerAttr>'}}
   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()
+  }
+}