[FIRRTL] Add domain verification to instance ops and domain support to instance choice ops

lib/Dialect/FIRRTL/FIRRTLInstanceImplementation.cpp

@@ -64,6 +64,13 @@ instance_like_impl::verifyReferencedModule(Operation *instanceOp,
         instanceOp->emitOpError("the number of result annotations should be "
                                 "equal to the number of results"));
 
+  auto domainInfo = instanceOp->getAttrOfType<ArrayAttr>("domainInfo");
+  if (domainInfo.size() != numExpected)
+    return emitNote(
+        instanceOp->emitOpError()
+        << "has a wrong number of port domain info attributes; expected "
+        << numExpected << ", got " << domainInfo.size());
+
   // Check that the port names match the referenced module.
   if (portNames != referencedModule.getPortNamesAttr()) {
     // We know there is an error, try to figure out whats wrong.
@@ -122,6 +129,17 @@ instance_like_impl::verifyReferencedModule(Operation *instanceOp,
     llvm_unreachable("should have found something wrong");
   }
 
+  // Check the domain info matches.
+  for (size_t i = 0; i < numResults; ++i) {
+    auto portDomainInfo = domainInfo[i];
+    auto modulePortDomainInfo = referencedModule.getDomainInfoAttrForPort(i);
+    if (portDomainInfo != modulePortDomainInfo)
+      return emitNote(instanceOp->emitOpError()
+                      << "domain info for " << portNames[i] << " must be "
+                      << modulePortDomainInfo << ", but got "
+                      << portDomainInfo);
+  }
+
   // Check that the instance op lists the correct layer requirements.
   auto instanceLayers = instanceOp->getAttrOfType<ArrayAttr>("layers");
   auto moduleLayers = referencedModule.getLayersAttr();

lib/Dialect/FIRRTL/FIRRTLOps.cpp

@@ -2924,6 +2924,13 @@ void InstanceChoiceOp::build(
     portAnnotationsAttr = builder.getArrayAttr(portAnnotations);
   }
 
+  // Create the domain info attribute.
+  ArrayAttr domainInfoAttr = defaultModule.getDomainInfoAttr();
+  if (domainInfoAttr.empty()) {
+    domainInfoAttr = builder.getArrayAttr(SmallVector<Attribute, 16>(
+        resultTypes.size(), builder.getArrayAttr({})));
+  }
+
   // Gather the module & case names.
   SmallVector<Attribute> moduleNames, caseNames;
   moduleNames.push_back(SymbolRefAttr::get(defaultModule.getModuleNameAttr()));
@@ -2934,14 +2941,14 @@ void InstanceChoiceOp::build(
     moduleNames.push_back(SymbolRefAttr::get(caseModule.getModuleNameAttr()));
   }
 
-  return build(
-      builder, result, resultTypes, builder.getArrayAttr(moduleNames),
-      builder.getArrayAttr(caseNames), builder.getStringAttr(name),
-      NameKindEnumAttr::get(builder.getContext(), nameKind),
-      defaultModule.getPortDirectionsAttr(), defaultModule.getPortNamesAttr(),
-      defaultModule.getDomainInfoAttr(), builder.getArrayAttr(annotations),
-      portAnnotationsAttr, defaultModule.getLayersAttr(),
-      innerSym ? hw::InnerSymAttr::get(innerSym) : hw::InnerSymAttr());
+  return build(builder, result, resultTypes, builder.getArrayAttr(moduleNames),
+               builder.getArrayAttr(caseNames), builder.getStringAttr(name),
+               NameKindEnumAttr::get(builder.getContext(), nameKind),
+               defaultModule.getPortDirectionsAttr(),
+               defaultModule.getPortNamesAttr(), domainInfoAttr,
+               builder.getArrayAttr(annotations), portAnnotationsAttr,
+               defaultModule.getLayersAttr(),
+               innerSym ? hw::InnerSymAttr::get(innerSym) : hw::InnerSymAttr());
 }
 
 std::optional<size_t> InstanceChoiceOp::getTargetResultIndex() {
@@ -3067,7 +3074,7 @@ ParseResult InstanceChoiceOp::parse(OpAsmParser &parser,
   }
 
   if (parseModulePorts(parser, /*hasSSAIdentifiers=*/false,
-                       /*supportsSymbols=*/false, /*supportsDomains=*/false,
+                       /*supportsSymbols=*/false, /*supportsDomains=*/true,
                        entryArgs, portDirections, portNames, portTypes,
                        portAnnotations, portSyms, portLocs, domains))
     return failure();

test/Dialect/FIRRTL/errors.mlir

@@ -3160,3 +3160,58 @@ firrtl.circuit "NonPropertyTypeInDomainField" {
   firrtl.domain @Foo ["bar", !firrtl.uint<1>]
   firrtl.module @NonPropertyTypeInDomainField() {}
 }
+
+// -----
+
+firrtl.circuit "WrongInstanceDomainInfo" {
+  firrtl.domain @ClockDomain
+  // expected-note @below {{original module declared here}}
+  firrtl.module @Foo(in %a: !firrtl.uint<1>) {}
+  firrtl.module @WrongInstanceDomainInfo() {
+    // expected-error @below {{'firrtl.instance' op has a wrong number of port domain info attributes; expected 1, got 0}}
+    %a = firrtl.instance foo  {domainInfo = []} @Foo(in a: !firrtl.uint<1>)
+  }
+}
+
+// -----
+
+firrtl.circuit "WrongInstanceDomainInfo" {
+  firrtl.domain @ClockDomain
+  // expected-note @below {{original module declared here}}
+  firrtl.module @Foo(
+    in %A : !firrtl.domain of @ClockDomain,
+    in %B : !firrtl.domain of @ClockDomain,
+    in %a : !firrtl.uint<1> domains [%A]
+  ) {}
+  firrtl.module @WrongInstanceDomainInfo() {
+  // expected-error @below {{op domain info for "a" must be [0 : ui32], but got [1 : ui32]}}
+    %foo_A, %foo_B, %foo_a = firrtl.instance foo @Foo(
+      in A : !firrtl.domain of @ClockDomain,
+      in B : !firrtl.domain of @ClockDomain,
+      in a : !firrtl.uint<1> domains [B]
+    )
+  }
+}
+
+// -----
+
+firrtl.circuit "WrongInstanceChoiceDomainInfo" {
+  firrtl.option @Platform {
+    firrtl.option_case @FPGA
+  }
+  firrtl.domain @ClockDomain
+  // expected-note @below {{original module declared here}}
+  firrtl.module @Foo(
+    in %A : !firrtl.domain of @ClockDomain,
+    in %B : !firrtl.domain of @ClockDomain,
+    in %a : !firrtl.uint<1> domains [%A]
+  ) {}
+  firrtl.module @WrongInstanceChoiceDomainInfo() {
+    // expected-error @below {{op domain info for "a" must be [0 : ui32], but got [1 : ui32]}}
+    %foo_A, %foo_B, %foo_a = firrtl.instance_choice foo @Foo alternatives @Platform { @FPGA -> @Foo } (
+      in A : !firrtl.domain of @ClockDomain,
+      in B : !firrtl.domain of @ClockDomain,
+      in a : !firrtl.uint<1> domains [B]
+    )
+  }
+}

test/Dialect/FIRRTL/round-trip.mlir

@@ -262,4 +262,18 @@ firrtl.module @DomainDefine(
   firrtl.domain.define %y, %x
 }
 
+firrtl.module private @DefaultTargetWithDomain(in %A: !firrtl.domain of @ClockDomain, in %clock: !firrtl.clock domains [%A]) {}
+firrtl.module private @FPGATargetWithDomain(in %A: !firrtl.domain of @ClockDomain, in %clock: !firrtl.clock domains [%A]) {}
+firrtl.module private @ASICTargetWithDomain(in %A: !firrtl.domain of @ClockDomain, in %clock: !firrtl.clock domains [%A]) {}
+
+// CHECK-LABEL: firrtl.module @InstanceChoiceWithDomain
+firrtl.module @InstanceChoiceWithDomain(in %A: !firrtl.domain of @ClockDomain, in %clock: !firrtl.clock domains [%A]) {
+  // CHECK:      %inst_A, %inst_clock = firrtl.instance_choice inst interesting_name @DefaultTargetWithDomain alternatives @Platform
+  // CHECK-SAME:   { @FPGA -> @FPGATargetWithDomain, @ASIC -> @ASICTargetWithDomain } (in A: !firrtl.domain of @ClockDomain, in clock: !firrtl.clock domains [A])
+  %inst_A, %inst_clock = firrtl.instance_choice inst interesting_name @DefaultTargetWithDomain alternatives @Platform
+    { @FPGA -> @FPGATargetWithDomain, @ASIC -> @ASICTargetWithDomain } (in A: !firrtl.domain of @ClockDomain, in clock: !firrtl.clock domains [A])
+  firrtl.matchingconnect %inst_clock, %clock : !firrtl.clock
+  firrtl.domain.define %inst_A, %A
+}
+
 }