[Mlir-commits] [mlir] 1ada304 - [mlir][OpenACC] Keep ThreadY active for inner-combine-fed worker reductions (#211696)

[llvmlistbot at llvm.org]

Author: Matsu
Date: 2026-07-24T06:37:35-07:00
New Revision: 1ada3044291aa3215b9deb2aeec3e952c13c2813

URL: https://github.com/llvm/llvm-project/commit/1ada3044291aa3215b9deb2aeec3e952c13c2813
DIFF: https://github.com/llvm/llvm-project/commit/1ada3044291aa3215b9deb2aeec3e952c13c2813.diff

LOG: [mlir][OpenACC] Keep ThreadY active for inner-combine-fed worker reductions (#211696)

Example:
```fortran
res = 0
!$cuf kernel do(2) <<< *, (32,8) >>> reduce(+:res)
do j2 = 1, n2
  do j1 = 1, n1
    res = res + a(j1, j2)
  end do
end do
```
In this code the reduction accumulator is per-(block_y, thread_y): each
worker row's shared slot is filled by an inner block-scoped combine, so
the rows hold distinct partials. The final combine into the result was
classified as not "worker-private" (block_y+thread_y into a global
dest), so it fell back to the ThreadY row-zero path and dropped every
worker but row 0 — a 2D SUM returned -1 instead of -4.
Fix: keep ThreadY active for a block_y+thread_y accumulator that is fed
by an inner block-scoped combine (distinct per-worker partials). A plain
worker accumulate still lowers to a worker-wide all_reduce that
broadcasts the total, so it correctly stays row-zero and is not
multiplied by the worker count.

Added: 
    

Modified: 
    mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
index 3bb78cd834b3e..d50d346ed9204 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
@@ -1276,6 +1276,30 @@ static bool hasUnsafeEffectsWhenBroadening(Operation *op) {
   return !op->hasTrait<OpTrait::HasRecursiveMemoryEffects>();
 }
 
+/// True when \p accumulator is the destination of a separate block-scoped
+/// combine, so it holds a distinct per-worker partial rather than a broadcast.
+static bool isFedByInnerBlockCombine(acc::PrivateLocalOp accumulator,
+                                     Operation *selfCombine) {
+  if (!accumulator)
+    return false;
+  for (Operation *user : accumulator.getResult().getUsers()) {
+    if (user == selfCombine)
+      continue;
+    auto combineOp = dyn_cast<acc::ReductionCombineOp>(user);
+    if (!combineOp ||
+        unwrapMemRefConversion(combineOp.getDestMemref()).getDefiningOp() !=
+            accumulator.getOperation())
+      continue;
+    SmallVector<mlir::acc::GPUParallelDimAttr> parDims =
+        getReductionCombineParDims(combineOp);
+    if (llvm::any_of(parDims, [](mlir::acc::GPUParallelDimAttr d) {
+          return d.isAnyBlock();
+        }))
+      return true;
+  }
+  return false;
+}
+
 /// Records whether \p combineOp requires ThreadY to remain active.
 static void classifyThreadYCombine(ThreadYBroadeningInfo &info,
                                    Operation *combineOp, Value src, Value dest,
@@ -1299,6 +1323,16 @@ static void classifyThreadYCombine(ThreadYBroadeningInfo &info,
     return;
   }
 
+  // A block_y+thread_y accumulator fed by an inner block-scoped combine holds
+  // a distinct partial per worker row, so its combine runs on every row; a
+  // plain worker accumulate broadcasts via all_reduce and stays row-zero.
+  if (hasThreadY && hasBlock &&
+      isThreadYPrivate(srcPrivate, /*allowBlock=*/true, computeRegion) &&
+      isFedByInnerBlockCombine(srcPrivate, combineOp)) {
+    info.hasActiveWorkerCombine = true;
+    return;
+  }
+
   info.hasExplicitInactiveCombine = true;
   if (!info.diagnosticOp)
     info.diagnosticOp = combineOp;