[FIRRTL] Add domain info verification to instance ops

Author: rwy7

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

@@ -2543,6 +2543,7 @@ void InstanceOp::build(OpBuilder &builder, OperationState &result,
   } else {
     portAnnotationsAttr = builder.getArrayAttr(portAnnotations);
   }
+
   ArrayAttr domainInfoAttr = module.getDomainInfoAttr();
   if (domainInfoAttr.empty()) {
     domainInfoAttr = builder.getArrayAttr(SmallVector<Attribute, 16>(
@@ -2881,6 +2882,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()));
@@ -2891,14 +2899,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() {
@@ -3024,7 +3032,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();
@@ -3165,10 +3173,9 @@ InstanceChoiceOp::erasePorts(OpBuilder &builder,
       builder, getLoc(), newResultTypes, getModuleNames(), getCaseNames(),
       getName(), getNameKind(),
       direction::packAttribute(getContext(), newPortDirections),
-      ArrayAttr::get(getContext(), newPortNames),
-      ArrayAttr::get(getContext(), newPortDomains), getAnnotationsAttr(),
-      ArrayAttr::get(getContext(), newPortAnnotations), getLayers(),
-      getInnerSymAttr());
+      ArrayAttr::get(getContext(), newPortNames), newPortDomains,
+      getAnnotationsAttr(), ArrayAttr::get(getContext(), newPortAnnotations),
+      getLayers(), getInnerSymAttr());
 
   for (unsigned oldIdx = 0, newIdx = 0, numOldPorts = getNumResults();
        oldIdx != numOldPorts; ++oldIdx) {

test/Dialect/FIRRTL/errors.mlir

@@ -3073,3 +3073,59 @@ firrtl.circuit "WrongNonDomainPortInfo" {
     in %a: !firrtl.uint<1>
   ) attributes {domainInfo = [["hello"]]} {}
 }
+
+// -----
+
+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]
+    )
+  }
+}