[Mlir-commits] [mlir] [mlir][acc] Report independent loops not parallelized due to unstructured control flow (PR #230066)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Oct 8 04:39:12 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-openacc
@llvm/pr-subscribers-mlir
Author: Kareem Ergawy (ergawy)
<details>
<summary>Changes</summary>
An independent acc loop whose body has unstructured control flow (for example a backward GOTO) cannot be represented as a structured loop, so it is executed sequentially. This happened silently: no remark was emitted for the loop, and the user only saw a serial kernel without a reason.
Emit a remark for such loops stating that they are not parallelized because their control flow could not be represented as a structured loop. This lets users identify loops that are asserted parallel but run sequentially.
Co-Authored-By: Claude
---
Full diff: https://github.com/llvm/llvm-project/pull/230066.diff
5 Files Affected:
- (modified) mlir/include/mlir/Dialect/OpenACC/OpenACC.h (+7)
- (modified) mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp (+13)
- (modified) mlir/lib/Dialect/OpenACC/Transforms/ACCEmitRemarksLoop.cpp (+16-3)
- (modified) mlir/test/Dialect/OpenACC/acc-compute-lowering-unstructured.mlir (+100)
- (modified) mlir/test/Dialect/OpenACC/acc-emit-remarks-loop.mlir (+22)
``````````diff
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
index 633be8159f2c6..a05d3d530358d 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h
@@ -221,6 +221,13 @@ static constexpr StringLiteral getCollapseCountAttrName() {
return StringLiteral("acc.collapse_count");
}
+/// Name for a unit attribute attached to the region an unstructured
+/// independent loop was converted to, recording that the loop could not be
+/// represented as a structured loop and is therefore not parallelized.
+static constexpr StringLiteral getUnstructuredIndependentLoopAttrName() {
+ return StringLiteral("acc.unstructured_independent_loop");
+}
+
static constexpr StringLiteral getVarNameAttrName() {
return VarNameAttr::name;
}
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp
index 84867cf58d5e6..f22d6f7b64afa 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCComputeLowering.cpp
@@ -346,10 +346,23 @@ class ACCLoopConversion : public OpRewritePattern<LoopOp> {
LogicalResult matchAndRewrite(LoopOp loopOp,
PatternRewriter &rewriter) const override {
if (loopOp.getUnstructured()) {
+ // An independent loop is asserted parallel but cannot be parallelized
+ // once it is lowered as a region of blocks. Record that, so the loss
+ // can be reported instead of silently running the loop sequentially.
+ bool lostIndependence =
+ loopOp.getDefaultOrDeviceTypeParallelism(deviceType) ==
+ LoopParMode::loop_independent &&
+ !isOpInSerialRegion(loopOp) &&
+ (isOpInComputeRegion(loopOp) ||
+ isSpecializedAccRoutine(
+ loopOp->getParentOfType<FunctionOpInterface>()));
auto executeRegion =
convertUnstructuredACCLoopToSCFExecuteRegion(loopOp, rewriter);
if (!executeRegion)
return failure();
+ if (lostIndependence)
+ executeRegion->setDiscardableAttr(
+ getUnstructuredIndependentLoopAttrName(), rewriter.getUnitAttr());
rewriter.replaceOp(loopOp, executeRegion);
return success();
}
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCEmitRemarksLoop.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCEmitRemarksLoop.cpp
index 77858609183d3..499e841195aa7 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCEmitRemarksLoop.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCEmitRemarksLoop.cpp
@@ -144,6 +144,16 @@ static void emitLoopMappingRemark(acc::ComputeRegionOp computeRegion,
DEBUG_TYPE);
}
+static void
+emitUnstructuredIndependentLoopRemark(Operation *op,
+ acc::OpenACCSupport &accSupport) {
+ accSupport.emitRemark(op,
+ "Loop is not parallelized: unstructured control "
+ "flow prevents representing this independent loop "
+ "as a structured loop",
+ DEBUG_TYPE);
+}
+
class ACCEmitRemarksLoop
: public acc::impl::ACCEmitRemarksLoopBase<ACCEmitRemarksLoop> {
public:
@@ -160,9 +170,12 @@ class ACCEmitRemarksLoop
if (!shouldEmitLoopRemarks(computeRegion))
return;
- computeRegion.getRegion().walk([&](LoopLikeOpInterface loopOp) {
- emitLoopMappingRemark(computeRegion, loopOp, accSupport, policy,
- gpuDimSeparator);
+ computeRegion.getRegion().walk([&](Operation *op) {
+ if (auto loopOp = dyn_cast<LoopLikeOpInterface>(op))
+ emitLoopMappingRemark(computeRegion, loopOp, accSupport, policy,
+ gpuDimSeparator);
+ else if (op->hasAttr(acc::getUnstructuredIndependentLoopAttrName()))
+ emitUnstructuredIndependentLoopRemark(op, accSupport);
});
});
}
diff --git a/mlir/test/Dialect/OpenACC/acc-compute-lowering-unstructured.mlir b/mlir/test/Dialect/OpenACC/acc-compute-lowering-unstructured.mlir
index bcfc066302cc4..98253edf1ff08 100644
--- a/mlir/test/Dialect/OpenACC/acc-compute-lowering-unstructured.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-compute-lowering-unstructured.mlir
@@ -13,6 +13,7 @@ func.func @parallel_unstructured_loop(%buf: memref<10xi32>) {
// CHECK-NOT: acc.par_width
// CHECK: acc.compute_region
// CHECK: scf.execute_region
+ // CHECK: } {acc.unstructured_independent_loop}
acc.parallel dataOperands(%dev : memref<10xi32>) {
acc.loop {
^entry:
@@ -32,3 +33,102 @@ func.func @parallel_unstructured_loop(%buf: memref<10xi32>) {
acc.copyout accPtr(%dev : memref<10xi32>) to varPtr(%buf : memref<10xi32>)
return
}
+
+// An unstructured seq loop in parallel is not an independent loop that
+// lost its parallelism, so the converted region is not marked.
+// CHECK-LABEL: func.func @parallel_unstructured_seq_loop
+// CHECK: scf.execute_region
+// CHECK-NOT: acc.unstructured_independent_loop
+// CHECK: return
+func.func @parallel_unstructured_seq_loop(%buf: memref<10xi32>) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %c10 = arith.constant 10 : index
+ %c1_i32 = arith.constant 1 : i32
+
+ %dev = acc.copyin varPtr(%buf : memref<10xi32>) -> memref<10xi32>
+ acc.parallel dataOperands(%dev : memref<10xi32>) {
+ acc.loop {
+ ^entry:
+ cf.br ^header(%c0 : index)
+ ^header(%iv: index):
+ %cond = arith.cmpi ult, %iv, %c10 : index
+ cf.cond_br %cond, ^body, ^exit
+ ^body:
+ memref.store %c1_i32, %dev[%iv] : memref<10xi32>
+ %iv_next = arith.addi %iv, %c1 : index
+ cf.br ^header(%iv_next : index)
+ ^exit:
+ acc.yield
+ } seq unstructured
+ acc.yield
+ }
+ acc.copyout accPtr(%dev : memref<10xi32>) to varPtr(%buf : memref<10xi32>)
+ return
+}
+
+// An unstructured independent loop in serial is not an independent loop that
+// lost its parallelism, so the converted region is not marked.
+// CHECK-LABEL: func.func @serial_unstructured_independent_loop
+// CHECK: scf.execute_region
+// CHECK-NOT: acc.unstructured_independent_loop
+// CHECK: return
+func.func @serial_unstructured_independent_loop(%buf: memref<10xi32>) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %c10 = arith.constant 10 : index
+ %c1_i32 = arith.constant 1 : i32
+
+ %dev = acc.copyin varPtr(%buf : memref<10xi32>) -> memref<10xi32>
+ acc.serial dataOperands(%dev : memref<10xi32>) {
+ acc.loop {
+ ^entry:
+ cf.br ^header(%c0 : index)
+ ^header(%iv: index):
+ %cond = arith.cmpi ult, %iv, %c10 : index
+ cf.cond_br %cond, ^body, ^exit
+ ^body:
+ memref.store %c1_i32, %dev[%iv] : memref<10xi32>
+ %iv_next = arith.addi %iv, %c1 : index
+ cf.br ^header(%iv_next : index)
+ ^exit:
+ acc.yield
+ } independent unstructured
+ acc.yield
+ }
+ acc.copyout accPtr(%dev : memref<10xi32>) to varPtr(%buf : memref<10xi32>)
+ return
+}
+
+// An unstructured auto loop in kernels is not an independent loop that
+// lost its parallelism, so the converted region is not marked.
+// CHECK-LABEL: func.func @kernels_unstructured_auto_loop
+// CHECK: scf.execute_region
+// CHECK-NOT: acc.unstructured_independent_loop
+// CHECK: return
+func.func @kernels_unstructured_auto_loop(%buf: memref<10xi32>) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %c10 = arith.constant 10 : index
+ %c1_i32 = arith.constant 1 : i32
+
+ %dev = acc.copyin varPtr(%buf : memref<10xi32>) -> memref<10xi32>
+ acc.kernels dataOperands(%dev : memref<10xi32>) {
+ acc.loop {
+ ^entry:
+ cf.br ^header(%c0 : index)
+ ^header(%iv: index):
+ %cond = arith.cmpi ult, %iv, %c10 : index
+ cf.cond_br %cond, ^body, ^exit
+ ^body:
+ memref.store %c1_i32, %dev[%iv] : memref<10xi32>
+ %iv_next = arith.addi %iv, %c1 : index
+ cf.br ^header(%iv_next : index)
+ ^exit:
+ acc.yield
+ } auto_ unstructured
+ acc.terminator
+ }
+ acc.copyout accPtr(%dev : memref<10xi32>) to varPtr(%buf : memref<10xi32>)
+ return
+}
diff --git a/mlir/test/Dialect/OpenACC/acc-emit-remarks-loop.mlir b/mlir/test/Dialect/OpenACC/acc-emit-remarks-loop.mlir
index 1b37317f19efc..e2a1eca986cdc 100644
--- a/mlir/test/Dialect/OpenACC/acc-emit-remarks-loop.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-emit-remarks-loop.mlir
@@ -174,3 +174,25 @@ func.func @acc_routine_gang_vector_loop() attributes {acc.specialized_routine =
}
return
}
+
+// -----
+
+// An independent loop that was lowered as an unstructured region is reported
+// as not parallelized instead of being skipped silently.
+// CHECK: remark: [Passed] openacc | Category:acc-emit-remarks-loop | Function=unstructured_independent_loop | Remark="Loop is not parallelized: unstructured control flow prevents representing this independent loop as a structured loop"
+func.func @unstructured_independent_loop() {
+ acc.kernel_environment {
+ acc.compute_region {
+ %flag = arith.constant true
+ scf.execute_region {
+ cf.cond_br %flag, ^bb1, ^bb2
+ ^bb1:
+ cf.br ^bb2
+ ^bb2:
+ scf.yield
+ } {acc.unstructured_independent_loop}
+ acc.yield
+ } <{origin = "acc.parallel"}>
+ }
+ return
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/230066
More information about the Mlir-commits
mailing list