[Mlir-commits] [mlir] [mlir][irdl] Introduce names in IRDL value lists (PR #123525)

[Fehr Mathieu]

=?utf-8?q?Théo?= Degioanni,=?utf-8?q?Théo?= Degioanni
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/123525 at github.com>


================
@@ -78,30 +81,104 @@ LogicalResult DialectOp::verify() {
   return success();
 }
 
-LogicalResult OperandsOp::verify() {
-  size_t numVariadicities = getVariadicity().size();
-  size_t numOperands = getNumOperands();
+LogicalResult OperationOp::verifyRegions() {
+  // Stores pairs of value kinds and the list of names of values of this kind in
+  // the operation.
+  SmallVector<std::tuple<StringRef, llvm::SmallDenseSet<StringRef>>> valueNames;
+
+  auto insertNames = [&](StringRef kind, ArrayAttr names) {
+    llvm::SmallDenseSet<StringRef> nameSet;
+    nameSet.reserve(names.size());
+    for (auto name : names)
+      nameSet.insert(llvm::cast<StringAttr>(name).getValue());
+    valueNames.emplace_back(kind, std::move(nameSet));
+  };
 
-  if (numOperands != numVariadicities)
-    return emitOpError()
-           << "the number of operands and their variadicities must be "
+  getBody().walk([&](Operation *op) {
+    TypeSwitch<Operation *>(op)
+        .Case<OperandsOp>(
+            [&](OperandsOp op) { insertNames("operands", op.getNames()); })
+        .Case<ResultsOp>(
+            [&](ResultsOp op) { insertNames("results", op.getNames()); })
+        .Case<RegionsOp>(
+            [&](RegionsOp op) { insertNames("regions", op.getNames()); });
+  });
+
+  // Verify that no two operand, result or region share the same name.
+  for (size_t i : llvm::seq(valueNames.size())) {
+    for (size_t j : llvm::seq(i + 1, valueNames.size())) {
+      auto &[lhs, lhsSet] = valueNames[i];
+      auto &[rhs, rhsSet] = valueNames[j];
+      llvm::set_intersect(lhsSet, rhsSet);
+      if (!lhsSet.empty())
+        return emitOpError("contains a value named '")
+               << *lhsSet.begin() << "' for both its " << lhs << " and " << rhs;
+    }
+  }
+
+  return success();
+}
+
+static LogicalResult verifyNames(Operation *op, StringRef kindName,
+                                 ArrayAttr names, size_t numOperands) {
+  if (numOperands != names.size())
+    return op->emitOpError()
+           << "the number of " << kindName
+           << "s and their names must be "
               "the same, but got "
-           << numOperands << " and " << numVariadicities << " respectively";
+           << numOperands << " and " << names.size() << " respectively";
+
+  DenseMap<StringRef, size_t> nameMap;
+  for (auto [i, name] : llvm::enumerate(names)) {
+    StringRef nameRef = llvm::cast<StringAttr>(name).getValue();
+    if (nameRef.size() == 0)
+      return op->emitOpError()
+             << "name of " << kindName << " number " << i << " is empty";
----------------
math-fehr wrote:

I've seen `operand #0` used somewhere, not sure if you prefer this or `number` spelled.

https://github.com/llvm/llvm-project/pull/123525