[Mlir-commits] [mlir] [mlir][OpenACC] Diagnose unmapped parallel loops in GPU lowering (PR #225641)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Oct 2 04:53:29 PDT 2026
https://github.com/leovancekang updated https://github.com/llvm/llvm-project/pull/225641
>From 5c3ef2f580457c4ddea3d34433d3d0378b98cf31 Mon Sep 17 00:00:00 2001
From: leovancekang <leovancekang at outlook.com>
Date: Tue, 29 Sep 2026 12:27:29 +0800
Subject: [PATCH 1/2] [mlir][OpenACC] Diagnose unmapped parallel loops in GPU
lowering
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.
---
.../Dialect/OpenACC/Transforms/ACCCGToGPU.cpp | 67 ++++++++------
...cg-to-gpu-attrless-parallel-ancestors.mlir | 87 +++++++++++++++++++
.../OpenACC/acc-cg-to-gpu-launch-mapping.mlir | 5 +-
3 files changed, 129 insertions(+), 30 deletions(-)
create mode 100644 mlir/test/Dialect/OpenACC/acc-cg-to-gpu-attrless-parallel-ancestors.mlir
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
index a7ac7b15d4925d..425ffe1689ae60 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
@@ -1996,7 +1996,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
@@ -2009,14 +2016,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.
@@ -2027,23 +2029,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 =
@@ -2055,7 +2055,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
@@ -2068,8 +2068,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()) {
@@ -2080,13 +2079,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;
@@ -2107,8 +2103,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);
}
}
@@ -4333,6 +4328,22 @@ 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 before acc-cg-to-gpu");
+ 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..26a8d3663eb5c1
--- /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 before acc-cg-to-gpu}}
+ 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 before acc-cg-to-gpu}}
+ 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 before acc-cg-to-gpu}}
+ 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
>From 6ea8aa9fe745e971bd95f3fcbad2e2c1a1c8be8c Mon Sep 17 00:00:00 2001
From: leovancekang <leovancekang at outlook.com>
Date: Fri, 2 Oct 2026 19:42:51 +0800
Subject: [PATCH 2/2] Remove redundant pass name from diagnostic
---
mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp | 3 +--
.../OpenACC/acc-cg-to-gpu-attrless-parallel-ancestors.mlir | 6 +++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
index 425ffe1689ae60..a5a454e12089c5 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCCGToGPU.cpp
@@ -4335,8 +4335,7 @@ class ACCCGToGPU : public acc::impl::ACCCGToGPUBase<ACCCGToGPU> {
if (!loop->getParentOfType<acc::ComputeRegionOp>() ||
mlir::acc::hasParDimsAttr(loop))
return WalkResult::advance();
- loop.emitOpError(
- "requires an 'acc.par_dims' attribute before acc-cg-to-gpu");
+ loop.emitOpError("requires an 'acc.par_dims' attribute");
return WalkResult::interrupt();
});
if (result.wasInterrupted()) {
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
index 26a8d3663eb5c1..3c3a6fc650cf83 100644
--- 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
@@ -10,7 +10,7 @@ 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 before acc-cg-to-gpu}}
+ // expected-error at +1 {{requires an 'acc.par_dims' attribute}}
scf.parallel (%i) = (%c0) to (%c1) step (%c1) {
}
} <{origin = "acc.parallel"}>
@@ -29,7 +29,7 @@ func.func @attrless_parallel_nested_in_if() {
%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 before acc-cg-to-gpu}}
+ // expected-error at +1 {{requires an 'acc.par_dims' attribute}}
scf.parallel (%j) = (%c0) to (%c1) step (%c1) {
}
}
@@ -48,7 +48,7 @@ module attributes {gpu.container_module} {
acc.compute_region {
%c0 = arith.constant 0 : index
%c1 = arith.constant 1 : index
- // expected-error at +1 {{requires an 'acc.par_dims' attribute before acc-cg-to-gpu}}
+ // expected-error at +1 {{requires an 'acc.par_dims' attribute}}
scf.parallel (%i) = (%c0) to (%c1) step (%c1) {
}
} <{origin = "acc.routine"}>
More information about the Mlir-commits
mailing list