[Mlir-commits] [mlir] [mlir][OpenACC] Fix tile clause element bounds for descending loops (PR #227185)
Vijay Kandiah
llvmlistbot at llvm.org
Mon Sep 28 20:46:36 PDT 2026
https://github.com/VijayKandiah created https://github.com/llvm/llvm-project/pull/227185
acc.loops could have negative steps, but ACCLoopTiling computed each element loop's bound assuming an ascending one. Both halves of that computation were wrong for a descending dimension: the inclusive edge subtracted one instead of adding one, and the bound was clamped with min instead of max.
For `DO j = n, floor, -1` under `tile(32)`, the last tile's bound came out as `min(floor, j - 33)`, which selects `j - 33` whenever it falls below the floor. The element loop then ran past the end of the iteration space, reading and writing outside the array.
With this change, we pick the clamp and the inclusive edge from the step's direction. When the sign is only known at runtime, we compute both and select on it.
The tiling tests had no coverage of negative steps or of inclusive upper bounds. This patch adds four new tests: descending, descending with an inclusive bound, ascending with an inclusive bound, and a runtime-signed step.
>From cf6264ed50fc6d6a2b51706f635466efe071f800 Mon Sep 17 00:00:00 2001
From: Vijay Kandiah <vkandiah at nvidia.com>
Date: Mon, 28 Sep 2026 20:40:21 -0700
Subject: [PATCH] [mlir][OpenACC] Fix tile clause element bounds for descending
loops
---
.../OpenACC/Transforms/ACCLoopTiling.cpp | 4 +
.../OpenACC/Utils/OpenACCUtilsTiling.cpp | 47 ++++++++--
.../test/Dialect/OpenACC/acc-loop-tiling.mlir | 88 +++++++++++++++++++
3 files changed, 130 insertions(+), 9 deletions(-)
diff --git a/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp b/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp
index d5865bc98727e..766d50fe54348 100644
--- a/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp
+++ b/mlir/lib/Dialect/OpenACC/Transforms/ACCLoopTiling.cpp
@@ -35,6 +35,10 @@
// }
// }
//
+// The example element loop bounds above assume ascending steps. For an
+// acc.loop with negative steps, a descending dimension would use
+// max(ub, %i+s*T) instead.
+//
// Gang/worker/vector attributes are distributed as follows:
// - gang: applied to tile loops
// - vector: applied to element loops
diff --git a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
index 32fd3957894fb..0d88404922cb6 100644
--- a/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
+++ b/mlir/lib/Dialect/OpenACC/Utils/OpenACCUtilsTiling.cpp
@@ -196,7 +196,9 @@ mlir::acc::tileACCLoops(mlir::acc::LoopOp tileLoop,
}
}
- // Compute the element-loop upper bounds min(origUB, origIV + scaledStep).
+ // Compute each element-loop upper bound by clamping the tile's far edge,
+ // origIV + scaledStep, to origUB. An ascending loop takes
+ // min(origUB, edge - 1), a descending one max(origUB, edge + 1).
rewriter.setInsertionPoint(tileLoop.getBody().getTerminator());
llvm::SmallVector<mlir::Value, 3> elemLBs, elemUBs, elemSteps;
llvm::SmallVector<mlir::Type, 3> elemIVTypes;
@@ -204,16 +206,43 @@ mlir::acc::tileACCLoops(mlir::acc::LoopOp tileLoop,
for (unsigned i = 0; i < tileCount; ++i) {
mlir::Value stepped =
mlir::arith::AddIOp::create(rewriter, loc, origIVs[i], scaledSteps[i]);
- mlir::Value newUB = stepped;
- if (inclusiveUBs[i]) {
- // Inclusive UB: min(origUB, origIV + (scaledStep - 1)).
+ mlir::Type stepTy = scaledSteps[i].getType();
+ std::optional<int64_t> constStep = mlir::getConstantIntValue(origSteps[i]);
+
+ // The inclusive edge is one iteration short of the next tile's start, in
+ // whichever direction the loop runs.
+ auto inclusiveEdge = [&](bool descending) -> mlir::Value {
+ if (!inclusiveUBs[i])
+ return stepped;
mlir::Value c1 = mlir::arith::ConstantOp::create(
- rewriter, loc, scaledSteps[i].getType(),
- rewriter.getIntegerAttr(scaledSteps[i].getType(), 1));
- newUB = mlir::arith::SubIOp::create(rewriter, loc, stepped, c1);
+ rewriter, loc, stepTy, rewriter.getIntegerAttr(stepTy, 1));
+ if (descending)
+ return mlir::arith::AddIOp::create(rewriter, loc, stepped, c1);
+ return mlir::arith::SubIOp::create(rewriter, loc, stepped, c1);
+ };
+
+ mlir::Value elemUB;
+ if (constStep && *constStep < 0) {
+ elemUB = mlir::arith::MaxSIOp::create(rewriter, loc, origUBs[i],
+ inclusiveEdge(/*descending=*/true));
+ } else if (constStep) {
+ elemUB = mlir::arith::MinSIOp::create(
+ rewriter, loc, origUBs[i], inclusiveEdge(/*descending=*/false));
+ } else {
+ // The sign is only known at run time, so compute both and select.
+ mlir::Type origStepTy = origSteps[i].getType();
+ mlir::Value zero = mlir::arith::ConstantOp::create(
+ rewriter, loc, origStepTy, rewriter.getIntegerAttr(origStepTy, 0));
+ mlir::Value isDescending = mlir::arith::CmpIOp::create(
+ rewriter, loc, mlir::arith::CmpIPredicate::slt, origSteps[i], zero);
+ mlir::Value descUB = mlir::arith::MaxSIOp::create(
+ rewriter, loc, origUBs[i], inclusiveEdge(/*descending=*/true));
+ mlir::Value ascUB = mlir::arith::MinSIOp::create(
+ rewriter, loc, origUBs[i], inclusiveEdge(/*descending=*/false));
+ elemUB = mlir::arith::SelectOp::create(rewriter, loc, isDescending,
+ descUB, ascUB);
}
- elemUBs.push_back(
- mlir::arith::MinSIOp::create(rewriter, loc, origUBs[i], newUB));
+ elemUBs.push_back(elemUB);
elemLBs.push_back(origIVs[i]);
elemSteps.push_back(origSteps[i]);
elemIVTypes.push_back(origIVs[i].getType());
diff --git a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
index ad23945df7687..6db2060124e9a 100644
--- a/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
+++ b/mlir/test/Dialect/OpenACC/acc-loop-tiling.mlir
@@ -216,3 +216,91 @@ func.func @body_with_nested_region(%arg0: memref<10xi32>) {
} independent
return
}
+
+// Test a descending loop: the element bound clamps the tile's far edge up to
+// the original bound, so it uses max rather than min.
+
+// CHECK-LABEL: func.func @descending_loop_tile
+// CHECK: %[[CM4:.*]] = arith.constant -4 : index
+// CHECK: acc.loop control(%[[IV:.*]] : index) = (%{{.*}} : index) to (%[[UB:.*]] : index) step (%[[CM4]] : index) {
+// CHECK: %[[EDGE:.*]] = arith.addi %[[IV]], %[[CM4]] : index
+// CHECK: %[[MAX_UB:.*]] = arith.maxsi %[[UB]], %[[EDGE]] : index
+// CHECK: acc.loop control(%{{.*}} : index) = (%[[IV]] : index) to (%[[MAX_UB]] : index) step (%{{.*}} : index) {
+func.func @descending_loop_tile(%arg0: memref<16xf32>, %start: index, %floor: index) {
+ %cm1 = arith.constant -1 : index
+ %c4 = arith.constant 4 : index
+ acc.loop tile({%c4 : index}) control(%i : index) = (%start : index) to (%floor : index) step (%cm1 : index) {
+ %val = arith.index_castui %i : index to i32
+ %fval = arith.sitofp %val : i32 to f32
+ memref.store %fval, %arg0[%i] : memref<16xf32>
+ acc.yield
+ } independent
+ return
+}
+
+// Test a descending loop with an inclusive upper bound: the inclusive edge
+// moves in the step's direction, so it adds one instead of subtracting.
+
+// CHECK-LABEL: func.func @descending_inclusive_tile
+// CHECK: %[[CM32:.*]] = arith.constant -32 : index
+// CHECK: acc.loop control(%[[IV:.*]] : index) = (%{{.*}} : index) to (%[[UB:.*]] : index) step (%[[CM32]] : index) {
+// CHECK: %[[STEPPED:.*]] = arith.addi %[[IV]], %[[CM32]] : index
+// CHECK: %[[C1:.*]] = arith.constant 1 : index
+// CHECK: %[[EDGE:.*]] = arith.addi %[[STEPPED]], %[[C1]] : index
+// CHECK: %[[MAX_UB:.*]] = arith.maxsi %[[UB]], %[[EDGE]] : index
+// CHECK: acc.loop control(%{{.*}} : index) = (%[[IV]] : index) to (%[[MAX_UB]] : index) step (%{{.*}} : index) {
+func.func @descending_inclusive_tile(%arg0: memref<128xf32>, %start: index, %floor: index) {
+ %cm1 = arith.constant -1 : index
+ %c32 = arith.constant 32 : index
+ acc.loop tile({%c32 : index}) control(%i : index) = (%start : index) to (%floor : index) step (%cm1 : index) {
+ %val = arith.index_castui %i : index to i32
+ %fval = arith.sitofp %val : i32 to f32
+ memref.store %fval, %arg0[%i] : memref<128xf32>
+ acc.yield
+ } inclusiveUpperbound(array<i1: true>) independent
+ return
+}
+
+// Test an ascending loop with an inclusive upper bound: uses min, and the edge
+// subtracts one.
+
+// CHECK-LABEL: func.func @ascending_inclusive_tile
+// CHECK: acc.loop control(%[[IV:.*]] : index) = (%{{.*}} : index) to (%[[UB:.*]] : index) step (%{{.*}} : index) {
+// CHECK: %[[STEPPED:.*]] = arith.addi %[[IV]], %{{.*}} : index
+// CHECK: %[[C1:.*]] = arith.constant 1 : index
+// CHECK: %[[EDGE:.*]] = arith.subi %[[STEPPED]], %[[C1]] : index
+// CHECK: %[[MIN_UB:.*]] = arith.minsi %[[UB]], %[[EDGE]] : index
+func.func @ascending_inclusive_tile(%arg0: memref<128xf32>, %lb: index, %ub: index) {
+ %c1 = arith.constant 1 : index
+ %c32 = arith.constant 32 : index
+ acc.loop tile({%c32 : index}) control(%i : index) = (%lb : index) to (%ub : index) step (%c1 : index) {
+ %val = arith.index_castui %i : index to i32
+ %fval = arith.sitofp %val : i32 to f32
+ memref.store %fval, %arg0[%i] : memref<128xf32>
+ acc.yield
+ } inclusiveUpperbound(array<i1: true>) independent
+ return
+}
+
+// Test a step whose sign is only known at run time: both clamps are computed
+// and selected on the sign.
+
+// CHECK-LABEL: func.func @runtime_step_tile
+// CHECK: acc.loop control(%[[IV:.*]] : index) = (%{{.*}} : index) to (%[[UB:.*]] : index) step (%{{.*}} : index) {
+// CHECK: %[[EDGE:.*]] = arith.addi %[[IV]], %{{.*}} : index
+// CHECK: %[[ZERO:.*]] = arith.constant 0 : index
+// CHECK: %[[IS_DESC:.*]] = arith.cmpi slt, %[[STEP:.*]], %[[ZERO]] : index
+// CHECK: %[[MAX_UB:.*]] = arith.maxsi %[[UB]], %[[EDGE]] : index
+// CHECK: %[[MIN_UB:.*]] = arith.minsi %[[UB]], %[[EDGE]] : index
+// CHECK: %[[SEL:.*]] = arith.select %[[IS_DESC]], %[[MAX_UB]], %[[MIN_UB]] : index
+// CHECK: acc.loop control(%{{.*}} : index) = (%[[IV]] : index) to (%[[SEL]] : index) step (%{{.*}} : index) {
+func.func @runtime_step_tile(%arg0: memref<16xf32>, %lb: index, %ub: index, %step: index) {
+ %c4 = arith.constant 4 : index
+ acc.loop tile({%c4 : index}) control(%i : index) = (%lb : index) to (%ub : index) step (%step : index) {
+ %val = arith.index_castui %i : index to i32
+ %fval = arith.sitofp %val : i32 to f32
+ memref.store %fval, %arg0[%i] : memref<16xf32>
+ acc.yield
+ } independent
+ return
+}
More information about the Mlir-commits
mailing list