[HW] Add port name accessors to HWInstanceLike (#7757)

HWInstanceLike is currently 'empty' and thus essentially the same as InstanceGraphInstanceOpInterface. On the other hand, HWModuleLike deals with port names and hw::InstanceOp caches them and verifies they match the referenced module's port names. This PR basically makes this behavior part of what it means to be an HWInstanceLike. If one has an instance operation that does not deal with port names, it's still possible to just make it an InstanceGraphInstanceOpInterface directly

include/circt/Dialect/HW/HWOpInterfaces.td

@@ -416,7 +416,93 @@ def HWEmittableModuleLike : OpInterface<"HWEmittableModuleLike", [HWModuleLike,
 def HWInstanceLike : OpInterface<"HWInstanceLike", [
     InstanceGraphInstanceOpInterface]> {
   let cppNamespace = "circt::hw";
-  let description = "Provide common  module information.";
+  let description = "Provide common instance information.";
+
+  // Note that the single-element getters and setters could be implemented as
+  // shared declarations using the interface methods that operate on the entire
+  // array or vice versa.  However, depending on how the names are stored in the
+  // instance operation, they can be implemented more efficiently by giving the
+  // operation the freedom to define all those methods (e.g., by avoiding
+  // construction of unnecessary intermedate array attributes).
+  let methods = [
+    InterfaceMethod<[{
+      Return the name of the specified input port or null if it cannot be
+      determined.
+    }], "::mlir::StringAttr", "getInputName", (ins "size_t":$idx),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      return ::circt::hw::instance_like_impl::getName(
+        $_op.getArgNames(), idx);
+    }]>,
+
+    InterfaceMethod<[{
+      Return the name of the specified result or null if it cannot be
+      determined.
+    }], "::mlir::StringAttr", "getOutputName", (ins "size_t":$idx),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      return ::circt::hw::instance_like_impl::getName(
+        $_op.getResultNames(), idx);
+    }]>,
+
+    InterfaceMethod<[{
+      Change the name of the specified input port.
+    }], "void", "setInputName",
+    (ins "size_t":$idx, "::mlir::StringAttr":$name),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      $_op.setArgNamesAttr(::circt::hw::instance_like_impl::updateName(
+        $_op.getArgNames(), idx, name));
+    }]>,
+
+    InterfaceMethod<[{
+      Change the name of the specified output port.
+    }], "void", "setOutputName",
+    (ins "size_t":$idx, "::mlir::StringAttr":$name),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      $_op.setResultNamesAttr(::circt::hw::instance_like_impl::updateName(
+        $_op.getResultNames(), idx, name));
+    }]>,
+
+    InterfaceMethod<[{
+      Return the names of all input ports. If the instance operation stores the
+      names in an ArrayAttr this can avoid attribute constructions.
+    }],
+    "::mlir::ArrayAttr", "getInputNames", (ins),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      return $_op.getArgNamesAttr();
+    }]>,
+
+    InterfaceMethod<[{
+      Return the name of all ouput ports. If the instance operation stores the
+      names in an ArrayAttr this can avoid attribute constructions.
+    }],
+    "::mlir::ArrayAttr", "getOutputNames", (ins),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      return $_op.getResultNamesAttr();
+    }]>,
+
+    InterfaceMethod<[{
+      Change the names of all input ports. If all names have to be changed, this
+      can avoid repeated intermediate attribute constructions.
+    }], "void", "setInputNames", (ins "::mlir::ArrayAttr":$names),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      $_op.setArgNamesAttr(names);
+    }]>,
+
+    InterfaceMethod<[{
+      Change the names of all output ports. If all names have to be changed, this
+      can avoid repeated intermediate attribute constructions.
+    }], "void", "setOutputNames", (ins "::mlir::ArrayAttr":$names),
+    /*methodBody=*/[{}],
+    /*defaultImplementation=*/[{
+      $_op.setResultNamesAttr(names);
+    }]>,
+  ];
 }
 
 def InnerRefNamespace : NativeOpTrait<"InnerRefNamespace">;

include/circt/Dialect/HW/HWStructure.td

@@ -400,38 +400,6 @@ class HWInstanceOpBase<string mnemonic, list<Trait> traits = []> :
   code extraInstanceClassDeclaration = [{}];
 
   let extraClassDeclaration = extraInstanceClassDeclaration # [{
-    /// Return the name of the specified input port or null if it cannot be
-    /// determined.
-    StringAttr getArgumentName(size_t idx) {
-      return instance_like_impl::getName(getArgNames(), idx);
-    }
-
-    /// Return the name of the specified result or null if it cannot be
-    /// determined.
-    StringAttr getResultName(size_t idx) {
-      return instance_like_impl::getName(getResultNames(), idx);
-    }
-
-    /// Change the name of the specified input port.
-    void setArgumentName(size_t i, StringAttr name) {
-      setInputNames(instance_like_impl::updateName(getArgNames(), i, name));
-    }
-
-    /// Change the name of the specified output port.
-    void setResultName(size_t i, StringAttr name) {
-      setOutputNames(instance_like_impl::updateName(getResultNames(), i, name));
-    }
-
-    /// Change the names of all the input ports.
-    void setInputNames(ArrayAttr names) {
-      setArgNamesAttr(names);
-    }
-
-    /// Change the names of all the result ports.
-    void setOutputNames(ArrayAttr names) {
-      setResultNamesAttr(names);
-    }
-
     /// Return the values for the port in port order.
     /// Note: The module ports may not be input, output ordered. This computes
     /// the port index to instance result/input Value mapping.
@@ -467,7 +435,7 @@ class HWInstanceOpBase<string mnemonic, list<Trait> traits = []> :
     /// attribute.
     void $cppClass::getAsmResultNames(OpAsmSetValueNameFn setNameFn) {
       ::circt::hw::instance_like_impl::getAsmResultNames(
-          setNameFn, getInstanceName(), getResultNames(), getResults());
+          setNameFn, getInstanceName(), getOutputNames(), getResults());
     }
 
     void $cppClass::getValues(SmallVectorImpl<Value> &values,
@@ -488,7 +456,7 @@ class HWInstanceOpBase<string mnemonic, list<Trait> traits = []> :
 def InstanceOp : HWInstanceOpBase<"instance", [HWInstanceLike]> {
   let summary = "Create an instance of a module";
   let description = [{
-    This represents an instance of a module. The inputs and results are
+    This represents an instance of a module. The inputs and outputs are
     the referenced module's inputs and outputs.  The `argNames` and
     `resultNames` attributes must match the referenced module.
 

lib/Conversion/ExportVerilog/PrepareForEmission.cpp

@@ -74,10 +74,8 @@ static bool shouldSpillWire(Operation &op, const LoweringOptions &options) {
 }
 
 static StringAttr getArgName(Operation *op, size_t idx) {
-  if (auto inst = dyn_cast<hw::InstanceOp>(op))
-    return inst.getArgumentName(idx);
-  else if (auto inst = dyn_cast<InstanceChoiceOp>(op))
-    return inst.getArgumentName(idx);
+  if (auto inst = dyn_cast<HWInstanceLike>(op))
+    return inst.getInputName(idx);
   return {};
 }
 
@@ -143,10 +141,8 @@ static void replacePortWithWire(ImplicitLocOpBuilder &builder, Operation *op,
 }
 
 static StringAttr getResName(Operation *op, size_t idx) {
-  if (auto inst = dyn_cast<hw::InstanceOp>(op))
-    return inst.getResultName(idx);
-  else if (auto inst = dyn_cast<InstanceChoiceOp>(op))
-    return inst.getResultName(idx);
+  if (auto inst = dyn_cast<HWInstanceLike>(op))
+    return inst.getOutputName(idx);
   return {};
 }
 

lib/Dialect/Arc/Transforms/LowerState.cpp

@@ -333,7 +333,7 @@ Value ModuleLowering::getAllocatedState(OpResult result) {
     alloc->setAttr(
         "name", builder.getStringAttr(
                     instOp.getInstanceName() + "/" +
-                    instOp.getResultName(result.getResultNumber()).getValue()));
+                    instOp.getOutputName(result.getResultNumber()).getValue()));
 
   // HACK: If the result comes from an op that has a "names" attribute, use that
   // as a name for the allocation. This should no longer be necessary once we

lib/Dialect/SV/Transforms/SVExtractTestCode.cpp

@@ -165,7 +165,7 @@ static StringAttr getNameForPort(Value val,
       auto index = cast<mlir::OpResult>(val).getResultNumber();
       SmallString<64> portName = inst.getInstanceName();
       portName += ".";
-      auto resultName = inst.getResultName(index);
+      auto resultName = inst.getOutputName(index);
       if (resultName && !resultName.getValue().empty())
         portName += resultName.getValue();
       else

lib/Tools/circt-bmc/ExternalizeRegisters.cpp

@@ -168,11 +168,10 @@ void ExternalizeRegistersPass::runOnOperation() {
           addedOutputNames[module.getSymNameAttr()].append(newOutputNames);
           initialValues[module.getSymNameAttr()].append(
               initialValues[instanceOp.getModuleNameAttr().getAttr()]);
-          SmallVector<Attribute> argNames(instanceOp.getArgNamesAttr().begin(),
-                                          instanceOp.getArgNamesAttr().end());
+          SmallVector<Attribute> argNames(
+              instanceOp.getInputNames().getValue());
           SmallVector<Attribute> resultNames(
-              instanceOp.getResultNamesAttr().begin(),
-              instanceOp.getResultNamesAttr().end());
+              instanceOp.getOutputNames().getValue());
 
           for (auto [input, name] : zip_equal(newInputs, newInputNames)) {
             instanceOp.getInputsMutable().append(