Update appendPorts interface, to combine input and output ports

include/circt/Dialect/HW/HWStructure.td

@@ -232,9 +232,10 @@ def HWModuleOp : HWModuleOpBase<"module",
  /// Verifies the body of the function.
  LogicalResult verifyBody();
 
- void appendPorts(
- ArrayRef<std::tuple<StringAttr, Type, Location>> insertInputs,
- ArrayRef<std::pair<StringAttr, Value>> insertOutputs);
+ /// Append ports to the module, use the `outputVals` to update the output op
+ /// and set the `inputVals` with the new block args added.
+ void appendPorts(ArrayRef<PortInfo> ports, ArrayRef<Value> outputVals,
+ SmallVectorImpl<Value> &inputVals);
  }];
 
  let hasCustomAssemblyFormat = 1;

lib/Dialect/HW/HWOps.cpp

@@ -503,9 +503,9 @@ OpFoldResult ParamValueOp::fold(FoldAdaptor adaptor) {
  return getValueAttr();
 }
 
-void HWModuleOp::appendPorts(
- ArrayRef<std::tuple<StringAttr, Type, Location>> insertInputs,
- ArrayRef<std::pair<StringAttr, Value>> insertOutputs) {
+void HWModuleOp::appendPorts(ArrayRef<PortInfo> ports,
+  ArrayRef<Value> outputVals,
+  SmallVectorImpl<Value> &inputVals) {
 
  HWModuleOp module = *this;
  auto *context = module->getContext();
@@ -514,57 +514,54 @@ void HWModuleOp::appendPorts(
 
  ArrayRef<ModulePort> oldPorts = oldType.getPorts();
 
- unsigned newPortsCount =
- oldPorts.size() + insertInputs.size() + insertOutputs.size();
+ unsigned newPortsCount = oldPorts.size() + ports.size();
 
  SmallVector<ModulePort> newPorts;
  newPorts.reserve(newPortsCount);
  newPorts.append(oldPorts.begin(), oldPorts.end());
 
- for (auto &[name, type, loc] : insertInputs) {
- body->addArgument(type, loc);
- newPorts.emplace_back(ModulePort({name, type, ModulePort::Input}));
- }
-
- if (!insertOutputs.empty()) {
+ SmallVector<Attribute> newResultLocs;
+ if (!outputVals.empty()) {
+ // Update the output op.
  auto oldResultLocs = module.getResultLocs();
- SmallVector<Attribute> newResultLocs;
- newResultLocs.reserve(
- (oldResultLocs.has_value() ? oldResultLocs->getValue().size() : 0) +
- insertOutputs.size());
  if (oldResultLocs.has_value())
  newResultLocs.append(oldResultLocs->getValue().begin(),
  oldResultLocs->getValue().end());
-
  auto outputOp = cast<OutputOp>(body->getTerminator());
  auto oldOperands = outputOp->getOperands();
 
- SmallVector<Value> newOperands;
- newOperands.reserve(oldOperands.size() + insertOutputs.size());
- newOperands.append(oldOperands.begin(), oldOperands.end());
-
- for (auto &[name, out] : insertOutputs) {
- newOperands.emplace_back(out);
- newPorts.emplace_back(
- ModulePort({name, out.getType(), ModulePort::Output}));
- newResultLocs.emplace_back(out.getLoc());
+ SmallVector<Value> newOutputOperands;
+ llvm::append_range(newOutputOperands, oldOperands);
+ llvm::append_range(newOutputOperands, outputVals);
+ outputOp->setOperands(newOutputOperands);
+ }
+ auto oldPortAttrs = module.getAllPortAttrs();
+ SmallVector<Attribute> newPortAttrs(oldPortAttrs);
+ inputVals.reserve(ports.size() - outputVals.size());
+
+ for (auto &portInfo : ports) {
+ assert(!portInfo.isInOut() && "can only handle input or output ports");
+ if (portInfo.isInput())
+ inputVals.push_back(body->addArgument(portInfo.type, portInfo.loc));
+ else if (portInfo.isOutput())
+ newResultLocs.emplace_back(portInfo.loc);
+ newPorts.push_back(portInfo);
+ if (portInfo.attrs) {
+ if (newPortAttrs.empty() || newPortAttrs.size() < newPortsCount)
+ newPortAttrs.append(newPortsCount, {});
+ newPortAttrs[newPorts.size() - 1] = portInfo.attrs;
  }
- outputOp->setOperands(newOperands);
- module.setResultLocsAttr(ArrayAttr::get(context, newResultLocs));
  }
 
+ if (!outputVals.empty())
+ module.setResultLocsAttr(ArrayAttr::get(context, newResultLocs));
+
  auto newType = ModuleType::get(context, newPorts);
 
  module.setModuleType(newType);
- auto oldPortAttrs = module.getPerPortAttrs();
- if (oldPortAttrs.has_value()) {
- auto portAttrs = oldPortAttrs->getValue();
- SmallVector<Attribute> newPortAttrs;
- newPortAttrs.reserve(newPortsCount);
- newPortAttrs.append(portAttrs.begin(), portAttrs.end());
- newPortAttrs.append(newPortsCount - oldPortAttrs->size(), {});
- }
+ module.setAllPortAttrs(newPortAttrs);
 }
+
 //===----------------------------------------------------------------------===//
 // HWModuleOp
 //===----------------------------------------------------------------------===/

lib/Dialect/Seq/Transforms/SinkClockGates.cpp

@@ -26,6 +26,7 @@
 #include "circt/Dialect/HW/HWInstanceGraph.h"
 #include "circt/Dialect/HW/HWOpInterfaces.h"
 #include "circt/Dialect/HW/HWOps.h"
+#include "circt/Dialect/HW/PortImplementation.h"
 #include "circt/Dialect/Seq/SeqOps.h"
 #include "circt/Dialect/Seq/SeqPasses.h"
 #include "circt/Dialect/Seq/SeqTypes.h"
@@ -61,19 +62,19 @@ struct SinkClockGatesPass
  /// Append input ports to the module and all its instances.
  /// Add the given values as operands to the `instance`. Add constant true op
  /// as input to the other instances of the module.
- void appendInputsToModuleAndInstances(
+ SmallVector<Value> appendInputsToModuleAndInstances(
  HWInstanceLike instance, ArrayRef<HWInstanceLike> moduleInstances,
  ArrayRef<std::pair<StringAttr, Value>> inputs, HWModuleOp refMod) {
 
  if (inputs.empty())
- return;
+ return {};
  SmallVector<Attribute> argNames;
  if (auto args = instance->getAttrOfType<ArrayAttr>("argNames"))
  argNames.insert(argNames.begin(), args.begin(), args.end());
 
  SmallVector<Value> newOperands;
  newOperands.reserve(inputs.size());
- SmallVector<std::tuple<StringAttr, Type, Location>> newInputs;
+ SmallVector<PortInfo> newInputs;
  newInputs.reserve(inputs.size());
  for (const auto &[name, value] : inputs) {
  // If no argNames attribute, donot add it.
@@ -83,7 +84,12 @@ struct SinkClockGatesPass
  newOperands.push_back(value);
  // Prepare the additional input ports that need to be added to the
  // referenced module.
- newInputs.emplace_back(name, value.getType(), value.getLoc());
+ PortInfo p;
+ p.name = name;
+ p.type = value.getType();
+ p.dir = ModulePort::Direction::Input;
+ p.loc = value.getLoc();
+ newInputs.push_back(p);
  }
  // Add the new operands to the instance.
  instance->insertOperands(instance->getNumOperands(), newOperands);
@@ -108,8 +114,10 @@ struct SinkClockGatesPass
  otherInstance->setAttr(
  "argNames", ArrayAttr::get(instance->getContext(), argNames));
  }
+ SmallVector<Value> inputPorts;
  // Update all the modules with the new input ports.
- refMod.appendPorts(newInputs, {});
+ refMod.appendPorts(newInputs, {}, inputPorts);
+ return inputPorts;
  }
 
  /// Get the referenced module for an instance, and add all the instances of
@@ -299,20 +307,20 @@ void SinkClockGatesPass::runOnOperation() {
  }
  if (enablePorts.empty())
  continue;
- unsigned oldNumInputs = inst->getNumOperands();
 
  // Now update the instance and all the referenced modules with the new
  // enable ports.
- appendInputsToModuleAndInstances(inst, moduleInstances, enablePorts,
-   refMod);
+ auto newEnableInputs = appendInputsToModuleAndInstances(
+ inst, moduleInstances, enablePorts, refMod);
  // Third phase, Once all the modules are updated with the additional enable
  // ports, duplicate the clock_gate op in the referenced module.
  auto *block = refMod.getBodyBlock();
  auto builder = mlir::OpBuilder::atBlockBegin(block);
+ assert(gatedClkPorts.size() == newEnableInputs.size());
  for (auto [index, clkPort] : llvm::enumerate(gatedClkPorts)) {
  auto clk = block->getArgument(clkPort);
  assert(isa<ClockType>(clk.getType()));
- auto enable = block->getArgument(oldNumInputs + index);
+ auto enable = newEnableInputs[index];
  auto clkGate = builder.create<ClockGateOp>(enable.getLoc(), clk, enable,
  Value(), hw::InnerSymAttr());
  // Replace all the original clock users with the gated clock.