Address comments.

Author: lialan

mlir/include/mlir/Interfaces/ParallelCombiningOpInterface.td

@@ -59,40 +59,40 @@ def InParallelOpInterface : OpInterface<"InParallelOpInterface"> {
 
 def ParallelCombiningOpInterface : OpInterface<"ParallelCombiningOpInterface"> {
   let description = [{
-    A parallel combining op is an operation performs parallel updates to
-    destination tensors within the context of a parent iterating operation.
+    A parallel combining op is an operation that models parallel contributions
+    to result tensors within the context of a parent iterating operation.
 
     This interface is designed for operations that need to coordinate parallel
-    insertions or updates to tensors that are being constructed or modified
-    across multiple parallel iterations. The "updated destination" refers to a
-    destination tensor that accumulates results from parallel computations,
-    where each parallel iteration may contribute a slice, element, or region
-    to the final result.
+    insertions or contributions to tensors that are being constructed across
+    multiple parallel iterations. The destination refers to a tensor value that
+    is assembled by aggregating results from parallel computations; each
+    parallel iteration may contribute a slice, element, or region to the final
+    result. No in-place mutation of tensors is implied.
 
     One significant use case for this interface is `tensor.parallel_insert_slice`
-    which allows parallel insertion of slices into a destination tensor. But with
-    this interface, other operations that perform similar parallel updates can
-    also be defined.
+    which allows parallel insertion of slices that are aggregated into a
+    destination tensor. With this interface, other operations that express
+    similar parallel contributions can also be defined.
 
-    This op works within an op  implementing the
-    `InParallelOpInterface` that specifies how the parallel results are combined.
+    This op works within an op implementing the `InParallelOpInterface` that
+    specifies how the parallel results are combined.
 
     Key semantics:
-    - The operation identifies destination tensors that will be updated
-      through the `getUpdatedDestinations` method
-    - Each parallel iteration may update elements or regions of the
-      destination tensor
+    - The operation identifies destination tensors to which iterations
+      contribute through the `getUpdatedDestinations` method
+    - Each parallel iteration may produce elements or regions that are
+      incorporated into the destination tensor
     - The parent iterating operation manages the coordination and ensures
-      proper synchronization of these updates
+      proper synchronization of these contributions
 
     Note: This interface does not verify itself, it is up to the implementing operation
     to verify the correctness of the op.
   }];
   let cppNamespace = "::mlir";
 
   let methods = [
-    InterfaceMethod<[{
-        Returns the list of values updated by this op.
+    InterfaceMethod<[{ 
+        Returns the list of destination values this op contributes to.
       }],
       /*retTy=*/"::mlir::MutableOperandRange",
       /*methodName=*/"getUpdatedDestinations",

mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

@@ -4142,11 +4142,10 @@ DiagnosedSilenceableFailure doit(RewriterBase &rewriter, OpTy target,
   }
 
   // If we are inside a `ParallelCombiningOp` region, temporarily set the
-  // insertion point outside: only ops implementing ParallelCombiningOpInterface are
-  // allowed in there.
-  if (isa<mlir::ParallelCombiningOpInterface>(target.getOperation())) {
+  // insertion point outside: only ops implementing ParallelCombiningOpInterface
+  // are allowed in there.
+  if (isa<mlir::ParallelCombiningOpInterface>(target.getOperation()))
     rewriter.setInsertionPoint(target->getParentOp());
-  }
 
   Value extracted = tensor::ExtractSliceOp::create(
       rewriter, target.getLoc(), target.getDest(), target.getMixedOffsets(),

mlir/lib/Dialect/SCF/IR/SCF.cpp

@@ -1674,12 +1674,7 @@ struct ForallOpIterArgsFolder : public OpRewritePattern<ForallOp> {
     for (OpResult result : forallOp.getResults()) {
       OpOperand *opOperand = forallOp.getTiedOpOperand(result);
       BlockArgument blockArg = forallOp.getTiedBlockArgument(opOperand);
-      SmallVector<Operation *> combiningOps =
-          forallOp.getCombiningOps(blockArg);
-      if ((result.use_empty() &&
-           llvm::all_of(combiningOps,
-                        [](Operation *op) { return op->use_empty(); })) ||
-          combiningOps.empty()) {
+      if (result.use_empty() || forallOp.getCombiningOps(blockArg).empty()) {
         resultToDelete.insert(result);
       } else {
         resultToReplace.push_back(result);
@@ -1981,7 +1976,8 @@ LogicalResult InParallelOp::verify() {
   for (Operation &op : getRegion().front().getOperations()) {
     auto inParallelOp = dyn_cast<ParallelCombiningOpInterface>(&op);
     if (!inParallelOp) {
-      return this->emitOpError("expected only ParallelCombiningOpInterface") << " ops";
+      return this->emitOpError("expected only ParallelCombiningOpInterface")
+             << " ops";
     }
 
     // Verify that inserts are into out block arguments.
@@ -2028,11 +2024,11 @@ OpResult InParallelOp::getParentResult(int64_t idx) {
 
 SmallVector<BlockArgument> InParallelOp::getDests() {
   SmallVector<BlockArgument> updatedDests;
-  for (auto &yieldingOp : getYieldingOps()) {
+  for (Operation &yieldingOp : getYieldingOps()) {
     auto inParallelOp = dyn_cast<ParallelCombiningOpInterface>(&yieldingOp);
     if (!inParallelOp)
       continue;
-    for (auto &updatedOperand : inParallelOp.getUpdatedDestinations())
+    for (OpOperand &updatedOperand : inParallelOp.getUpdatedDestinations())
       updatedDests.push_back(cast<BlockArgument>(updatedOperand.get()));
   }
   return updatedDests;

mlir/lib/Dialect/Tensor/IR/TensorOps.cpp

@@ -3846,8 +3846,7 @@ OpFoldResult PadOp::fold(FoldAdaptor) {
 //===----------------------------------------------------------------------===//
 
 OpResult ParallelInsertSliceOp::getTiedOpResult() {
-  InParallelOpInterface parallelCombiningParent =
-      getParallelCombiningParent();
+  InParallelOpInterface parallelCombiningParent = getParallelCombiningParent();
   for (const auto &it :
        llvm::enumerate(parallelCombiningParent.getYieldingOps())) {
     Operation &nextOp = it.value();
@@ -3942,8 +3941,8 @@ MutableOperandRange ParallelInsertSliceOp::getUpdatedDestinations() {
 
 Operation *ParallelInsertSliceOp::getIteratingParent() {
   // Return the parent InParallelOpInterface's parent
-  if (auto combiningOp = dyn_cast<InParallelOpInterface>(
-          getOperation()->getParentOp())) {
+  if (auto combiningOp =
+          dyn_cast<InParallelOpInterface>(getOperation()->getParentOp())) {
     return combiningOp->getParentOp();
   }
   return nullptr;

mlir/lib/Dialect/Tensor/Transforms/FoldTensorSubsetOps.cpp

@@ -215,9 +215,9 @@ struct InsertSliceOfInsertSliceFolder : public OpRewritePattern<OpTy> {
                                         sourceInsertSliceOp.getMixedSizes(),
                                         droppedDims, resolvedSizes);
 
-    // If we are inside a ParallelCombining region, temporarily set the insertion
-    // point outside: only ops of ParallelCombiningOpInterface are allowed in
-    // there.
+    // If we are inside a ParallelCombining region, temporarily set the
+    // insertion point outside: only ops of ParallelCombiningOpInterface are
+    // allowed in there.
     if (isa<mlir::ParallelCombiningOpInterface>(insertSliceOp.getOperation())) {
       rewriter.setInsertionPoint(insertSliceOp->getParentOp());
     }