[Mlir-commits] [mlir] 9919736 - [mlir][OpenACC] Diagnose unmapped parallel loops in GPU lowering (#225641)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Oct 2 10:50:18 PDT 2026
Author: leovance
Date: 2026-10-02T17:50:10Z
New Revision: 9919736e294ec0dd8cac981088911884aad9500d
URL: https://github.com/llvm/llvm-project/commit/9919736e294ec0dd8cac981088911884aad9500d
DIFF: https://github.com/llvm/llvm-project/commit/9919736e294ec0dd8cac981088911884aad9500d.diff
LOG: [mlir][OpenACC] Diagnose unmapped parallel loops in GPU lowering (#225641)
ACCCGToGPU requires parallel dimensions to be assigned before lowering.
Diagnose scf.parallel operations without acc.par_dims inside
acc.compute_region before rewriting.
Keep barrier ancestor lookups within the current compute region so
enclosing host loops are not treated as GPU execution scopes.
Add diagnostic coverage for unmapped parallel loops in func.func and
gpu.func, including nested regions, and verify that surrounding host
loops remain valid. Give the launch-argument remapping test explicit
sequential dimensions.
Added:
mlir/test/Dialect/OpenACC/acc-cg-to-gpu-attrless-parallel-ancestors.mlir
Modified:
mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
mlir/test/Dialect/OpenACC/acc-cg-to-gpu-launch-mapping.mlir
Removed:
################################################################################
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
index 6bf9f43f5f9b22..c74549f4411cbf 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
@@ -2006,7 +2006,14 @@ static bool hasTrailingSideEffectSiblings(Operation *loopOp) {
// the block-level (or worker/thread-y) ancestor and insert the barrier
// there, handling gang-redundant init loops and grid-stride remainders.
void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
- scf::ParallelOp wsLoop = loopOp->getParentOfType<scf::ParallelOp>();
+ // Host loops enclosing the compute region are not GPU execution scopes.
+ auto getParentParallel = [&](Operation *op) -> scf::ParallelOp {
+ scf::ParallelOp parent = op->getParentOfType<scf::ParallelOp>();
+ if (parent && computeRegion->isAncestor(parent))
+ return parent;
+ return {};
+ };
+ scf::ParallelOp wsLoop = getParentParallel(loopOp);
if (!wsLoop) {
// loopOp is a worksharing loop at the kernel-body top level (no enclosing
// parallel loop). When it writes gang-private shared memory
@@ -2019,14 +2026,9 @@ void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
return;
}
- bool parentIsSeq = false;
- if (mlir::acc::GPUParallelDimsAttr wsParDims =
- mlir::acc::getParDimsAttr(wsLoop)) {
- if (wsParDims.getArray().size() == 1 &&
- wsParDims.getArray().front().isSeq()) {
- parentIsSeq = true;
- }
- }
+ mlir::acc::GPUParallelDimsAttr wsParDims = mlir::acc::getParDimsAttr(wsLoop);
+ bool parentIsSeq =
+ wsParDims.getArray().size() == 1 && wsParDims.getArray().front().isSeq();
if (parentIsSeq) {
// loopOp is nested inside a sequential parent loop.
@@ -2037,23 +2039,21 @@ void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
loopOp->walk([&](scf::ParallelOp innerPar) -> WalkResult {
if (innerPar.getOperation() == loopOp)
return WalkResult::advance();
- if (mlir::acc::GPUParallelDimsAttr dims =
- mlir::acc::getParDimsAttr(innerPar)) {
- for (auto d : dims.getArray()) {
- if (d.isThreadX() || d.isThreadY()) {
- hasThreadSubLoop = true;
- return WalkResult::interrupt();
- }
+ mlir::acc::GPUParallelDimsAttr dims = mlir::acc::getParDimsAttr(innerPar);
+ for (auto d : dims.getArray()) {
+ if (d.isThreadX() || d.isThreadY()) {
+ hasThreadSubLoop = true;
+ return WalkResult::interrupt();
}
}
return WalkResult::advance();
});
if (!hasThreadSubLoop)
return;
- scf::ParallelOp threadLoop = wsLoop->getParentOfType<scf::ParallelOp>();
+ scf::ParallelOp threadLoop = getParentParallel(wsLoop);
if (!threadLoop)
return;
- scf::ParallelOp blockLoop = threadLoop->getParentOfType<scf::ParallelOp>();
+ scf::ParallelOp blockLoop = getParentParallel(threadLoop);
if (!blockLoop)
return;
mlir::acc::GPUParallelDimsAttr parDimsAttr =
@@ -2065,7 +2065,7 @@ void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
// Parent is a non-sequential parallel loop. Walk up to find the block-level
// ancestor and insert a barrier there.
- scf::ParallelOp seqLoop = wsLoop->getParentOfType<scf::ParallelOp>();
+ scf::ParallelOp seqLoop = getParentParallel(wsLoop);
if (!seqLoop) {
// wsLoop is a worksharing loop directly under the compute region with no
// gang ancestor: a gang-redundant init loop (e.g. a thread-level loop that
@@ -2078,8 +2078,7 @@ void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
emitGPUBarrierWorkgroup(rewriter, loopOp->getLoc());
return;
}
- if (scf::ParallelOp outerParLoop =
- seqLoop->getParentOfType<scf::ParallelOp>()) {
+ if (scf::ParallelOp outerParLoop = getParentParallel(seqLoop)) {
mlir::acc::GPUParallelDimsAttr parDimsAttr =
mlir::acc::getParDimsAttr(outerParLoop);
if (parDimsAttr.hasOnlyBlockLevel()) {
@@ -2090,13 +2089,10 @@ void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
// outerParLoop is a sequential grid-stride remainder of a partitioned
// gang loop, not the gang. Walk past the remainder(s) to the block-level
// gang and barrier there.
- for (Operation *gangLoop =
- outerParLoop->getParentOfType<scf::ParallelOp>();
- gangLoop; gangLoop = gangLoop->getParentOfType<scf::ParallelOp>()) {
+ for (scf::ParallelOp gangLoop = getParentParallel(outerParLoop); gangLoop;
+ gangLoop = getParentParallel(gangLoop)) {
mlir::acc::GPUParallelDimsAttr gangDims =
mlir::acc::getParDimsAttr(gangLoop);
- if (!gangDims)
- break;
if (gangDims.hasOnlyBlockLevel()) {
createBarrier(loopOp->getLoc(), gangDims);
break;
@@ -2117,8 +2113,7 @@ void ACCCGToGPULowering::createBarrierAfterSeqLoop(Operation *loopOp) {
// not need one here.
mlir::acc::GPUParallelDimsAttr parDimsAttr =
mlir::acc::getParDimsAttr(seqLoop);
- if (parDimsAttr && parDimsAttr.hasOnlyBlockLevel() &&
- mayWriteSharedMemory(loopOp)) {
+ if (parDimsAttr.hasOnlyBlockLevel() && mayWriteSharedMemory(loopOp)) {
createBarrier(loopOp->getLoc(), parDimsAttr);
}
}
@@ -4356,6 +4351,21 @@ class ACCCGToGPU : public acc::impl::ACCCGToGPUBase<ACCCGToGPU> {
assert(deviceType != mlir::acc::DeviceType::Host &&
deviceType != mlir::acc::DeviceType::Multicore &&
"ACCCGToGPU only supports GPU device types");
+
+ // Validate parallel-loop mappings before any compute region is rewritten.
+ // Loops outside compute regions do not require GPU mappings.
+ WalkResult result = funcOp->walk([](scf::ParallelOp loop) {
+ if (!loop->getParentOfType<acc::ComputeRegionOp>() ||
+ mlir::acc::hasParDimsAttr(loop))
+ return WalkResult::advance();
+ loop.emitOpError("requires an 'acc.par_dims' attribute");
+ return WalkResult::interrupt();
+ });
+ if (result.wasInterrupted()) {
+ signalPassFailure();
+ return;
+ }
+
ACCCGToGPUOptions options;
options.deviceType = deviceType;
options.maxWorkgroupSharedMemory = maxWorkgroupSharedMemory;
diff --git a/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-attrless-parallel-ancestors.mlir b/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-attrless-parallel-ancestors.mlir
new file mode 100644
index 00000000000000..3c3a6fc650cf83
--- /dev/null
+++ b/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-attrless-parallel-ancestors.mlir
@@ -0,0 +1,87 @@
+// RUN: mlir-opt %s --pass-pipeline="builtin.module(func.func(acc-cg-to-gpu),gpu.module(gpu.func(acc-cg-to-gpu)))" \
+// RUN: --split-input-file --verify-diagnostics | FileCheck %s \
+// RUN: --implicit-check-not=acc.compute_region --implicit-check-not=gpu.barrier
+
+// Parallel dimensions must be assigned before lowering a compute region.
+// Reject an unmapped parallel loop even without a nested sequential loop that
+// would otherwise trigger a barrier lookup.
+
+func.func @attrless_parallel_in_compute_region() {
+ acc.compute_region {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ // expected-error at +1 {{requires an 'acc.par_dims' attribute}}
+ scf.parallel (%i) = (%c0) to (%c1) step (%c1) {
+ }
+ } <{origin = "acc.parallel"}>
+ return
+}
+
+// -----
+
+// A mapped enclosing loop does not make an unmapped nested loop valid. Check
+// recursively through non-loop regions as well.
+
+func.func @attrless_parallel_nested_in_if() {
+ acc.compute_region {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %true = arith.constant true
+ scf.parallel (%i) = (%c0) to (%c1) step (%c1) {
+ scf.if %true {
+ // expected-error at +1 {{requires an 'acc.par_dims' attribute}}
+ scf.parallel (%j) = (%c0) to (%c1) step (%c1) {
+ }
+ }
+ } {acc.par_dims = #acc<par_dims[block_x]>}
+ } <{origin = "acc.parallel"}>
+ return
+}
+
+// -----
+
+// The same input requirement applies to compute regions in GPU functions.
+
+module attributes {gpu.container_module} {
+ gpu.module @device {
+ gpu.func @attrless_parallel_in_gpu_func() {
+ acc.compute_region {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ // expected-error at +1 {{requires an 'acc.par_dims' attribute}}
+ scf.parallel (%i) = (%c0) to (%c1) step (%c1) {
+ }
+ } <{origin = "acc.routine"}>
+ gpu.return
+ }
+ }
+}
+
+// -----
+
+// Parallel loops outside the compute region do not need GPU mappings. Barrier
+// lookup for the plain sequential loop must not inspect these host ancestors.
+
+// CHECK-LABEL: func.func @attrless_host_parallel_ancestors
+// CHECK: scf.parallel
+// CHECK-NEXT: scf.parallel
+// CHECK-NEXT: scf.parallel
+// CHECK: gpu.launch
+// CHECK: scf.for
+func.func @attrless_host_parallel_ancestors() {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ scf.parallel (%i) = (%c0) to (%c1) step (%c1) {
+ scf.parallel (%j) = (%c0) to (%c1) step (%c1) {
+ scf.parallel (%k) = (%c0) to (%c1) step (%c1) {
+ acc.compute_region {
+ %c0_inner = arith.constant 0 : index
+ %c1_inner = arith.constant 1 : index
+ scf.for %l = %c0_inner to %c1_inner step %c1_inner {
+ }
+ } <{origin = "acc.parallel"}>
+ }
+ }
+ }
+ return
+}
diff --git a/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-launch-mapping.mlir b/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-launch-mapping.mlir
index 646f6efab322e3..e74e05157ad9ec 100644
--- a/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-launch-mapping.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-cg-to-gpu-launch-mapping.mlir
@@ -244,6 +244,7 @@ func.func @empty_all_known_launch_arg() {
// CHECK: %[[gdimz:.*]] = gpu.grid_dim z
// CHECK: arith.addi %[[gdimx]], %[[gdimy]] : index
// CHECK: arith.subi %[[gdimz]], %c2{{.*}} : index
+// Keep the loops sequential: this test checks launch-argument remapping.
func.func @using_block_args(%arr : memref<?xf32>) {
%c2_pw = arith.constant 2 : index
%c4_pw = arith.constant 4 : index
@@ -269,8 +270,8 @@ func.func @using_block_args(%arr : memref<?xf32>) {
scf.parallel (%arg9) = (%22) to (%23) step (%24) {
memref.store %cst, %arg10[%arg9] : memref<?xf32>
scf.reduce
- }
- }
+ } {acc.par_dims = #acc<par_dims[sequential]>}
+ } {acc.par_dims = #acc<par_dims[sequential]>}
acc.yield
} <{origin = "acc.parallel"}>
return
More information about the Mlir-commits
mailing list