[Mlir-commits] [mlir] [MLIR] Add `InParallelOpInterface` for parallel combining operations (PR #157736)

[Quinn Dawkins]

================
@@ -10,18 +10,47 @@
 
 using namespace mlir;
 
+/// Include the definitions of the interface.
+#include "mlir/Interfaces/ParallelCombiningOpInterface.cpp.inc"
+
 //===----------------------------------------------------------------------===//
-// ParallelCombiningOpInterface
+// InParallelOpInterface
 //===----------------------------------------------------------------------===//
 
+// TODO: Catch-22 with interface methods used to verify means methods can't
+// assume the impl is valid.
+LogicalResult mlir::detail::verifyInParallelOpInterface(Operation *op) {
+  auto inParallel = cast<InParallelOpInterface>(op);
+  auto parent = inParallel.getIteratingParent();
+  if (!parent) {
+    return op->emitError(
+        "in_parallel interface op must have an iterating parent");
+  }
+
+  // Simple verification without requiring ParallelIterationOpInterface
+  // Just check that updated destinations are block arguments
+  for (OpOperand &updatedValue : inParallel.getUpdatedDestinations()) {
+    auto bbArg = dyn_cast<BlockArgument>(updatedValue.get());
+    if (!bbArg) {
+      return op->emitError("updating a non block argument");
+    }
+  }
+  return success();
+}
----------------
qedawkins wrote:

I would just drop verifier from the new interface and advise implementers of the interface to do proper verification themselves. It's here for improved verification, but like you've noticed the TODO makes it not worth it.

> 3. Where is this restriction on block operand coming from? Where is it documented and what are the implications? (it also needs a test)

This is not a new requirement. `scf.forall` relies on terminator ops inserting directly into its tied `shared_outs` block argument to construct its result tensor. As a result, if anything ends up between the `shared_outs` and the destination of a `parallel_insert_slice` today, the only reasonable interpretation is that the op returns undefined values. For example,

```
%0 = scf.forall shared_outs(%dest = %init) {
  %1 = tensor.expand_shape %dest
  %2 = ... // some thread local thing
  scf.forall.in_parallel {
    tensor.parallel_insert_slice %2 into %1
  }
}
```

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