[Mlir-commits] [mlir] [mlir][SCFToGPU] Fix crash when deriving a static upper bound (PR #224262)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Thu Sep 17 04:06:31 PDT 2026
https://github.com/AkunCube created https://github.com/llvm/llvm-project/pull/224262
`deriveStaticUpperBound` signalled failure with a null `Value`, but the `arith.muli` case queried the defining op of that value directly:
```c++
if (auto lhs = deriveStaticUpperBound(multiplyOp.getOperand(0), rewriter)
.getDefiningOp<arith::ConstantIndexOp>())
```
`Value::getDefiningOp()` casts to `OpResult` with `llvm::dyn_cast`, which asserts on a null value. This is reachable for a nested `scf.parallel` whose bound is an `arith.muli` of a value with no static bound, such as `arith.muli %iv, %c1` in the outer loop: the pass derives a bound for the inner loop, recurses into the `arith.muli`, fails on the induction variable, and
dereferences the null result.
Return `FailureOr<int64_t>` and let the caller materialize the constant. The function becomes a pure query with no `rewriter` parameter and no side effects, which also removes two redundant `getDefiningOp<ConstantIndexOp>()` re-queries in the `arith.muli` case.
A bound that previously reused an existing `arith.constant` op now materializes an equivalent one; the value is identical and canonicalization merges it, but one CHECK line in parallel_loop.mlir was updated. Adds a regression test for the
case above.
Fixes: #224211
>From 584f964ccc7ad5fd7138b0aad915acfd9551c4a3 Mon Sep 17 00:00:00 2001
From: cubehan3 <cubehan3 at gmail.com>
Date: Thu, 17 Sep 2026 09:47:57 +0000
Subject: [PATCH] [mlir][SCFToGPU] Fix crash when deriving a static upper bound
---
mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp | 49 ++++++++++---------
.../Conversion/SCFToGPU/parallel_loop.mlir | 40 ++++++++++++++-
2 files changed, 64 insertions(+), 25 deletions(-)
diff --git a/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp b/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
index 944e535c08a18..64ef74aceac66 100644
--- a/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
+++ b/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
@@ -26,6 +26,7 @@
#include "mlir/IR/Builders.h"
#include "mlir/IR/IRMapping.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
+#include "mlir/Support/LLVM.h"
#include "mlir/Transforms/DialectConversion.h"
#include "mlir/Transforms/RegionUtils.h"
#include "llvm/ADT/DenseSet.h"
@@ -316,44 +317,42 @@ struct ParallelToGpuLaunchLowering : public OpRewritePattern<ParallelOp> {
/// Tries to derive a static upper bound from the defining operation of
/// `upperBound`.
-static Value deriveStaticUpperBound(Value upperBound,
- PatternRewriter &rewriter) {
+static FailureOr<int64_t> deriveStaticUpperBound(Value upperBound) {
if (auto op = upperBound.getDefiningOp<arith::ConstantIndexOp>()) {
- return op;
+ return op.value();
}
if (auto minOp = upperBound.getDefiningOp<AffineMinOp>()) {
for (const AffineExpr &result : minOp.getMap().getResults()) {
- if (auto constExpr = dyn_cast<AffineConstantExpr>(result)) {
- return arith::ConstantIndexOp::create(rewriter, minOp.getLoc(),
- constExpr.getValue());
- }
+ if (auto constExpr = dyn_cast<AffineConstantExpr>(result))
+ return constExpr.getValue();
}
}
if (auto minOp = upperBound.getDefiningOp<arith::MinSIOp>()) {
for (Value operand : {minOp.getLhs(), minOp.getRhs()}) {
- if (auto staticBound = deriveStaticUpperBound(operand, rewriter))
- return staticBound;
+ FailureOr<int64_t> staticBound = deriveStaticUpperBound(operand);
+ if (succeeded(staticBound))
+ return *staticBound;
}
}
if (auto multiplyOp = upperBound.getDefiningOp<arith::MulIOp>()) {
- if (auto lhs = deriveStaticUpperBound(multiplyOp.getOperand(0), rewriter)
- .getDefiningOp<arith::ConstantIndexOp>())
- if (auto rhs = deriveStaticUpperBound(multiplyOp.getOperand(1), rewriter)
- .getDefiningOp<arith::ConstantIndexOp>()) {
- // Assumptions about the upper bound of minimum computations no longer
- // work if multiplied by mixed signs, so abort in this case.
- if ((lhs.value() < 0) != (rhs.value() < 0))
- return {};
-
- return arith::ConstantIndexOp::create(rewriter, multiplyOp.getLoc(),
- lhs.value() * rhs.value());
- }
+ FailureOr<int64_t> lhs = deriveStaticUpperBound(multiplyOp.getOperand(0));
+ if (failed(lhs))
+ return failure();
+ FailureOr<int64_t> rhs = deriveStaticUpperBound(multiplyOp.getOperand(1));
+ if (failed(rhs))
+ return failure();
+ // Assumptions about the upper bound of minimum computations no longer
+ // work if multiplied by mixed signs, so abort in this case.
+ if ((lhs.value() < 0) != (rhs.value() < 0))
+ return failure();
+
+ return lhs.value() * rhs.value();
}
- return {};
+ return failure();
}
static bool isMappedToProcessor(gpu::Processor processor) {
@@ -499,13 +498,15 @@ static LogicalResult processParallelLoop(
PatternRewriter::InsertionGuard guard(rewriter);
rewriter.setInsertionPoint(launchOp);
if (!boundIsPrecise) {
- upperBound = deriveStaticUpperBound(upperBound, rewriter);
- if (!upperBound) {
+ FailureOr<int64_t> staticBound = deriveStaticUpperBound(upperBound);
+ if (failed(staticBound)) {
return rewriter.notifyMatchFailure(
parallelOp,
"cannot derive loop-invariant upper bound for number of"
"iterations");
}
+ upperBound = arith::ConstantIndexOp::create(
+ rewriter, upperBound.getLoc(), *staticBound);
}
// Compute the number of iterations needed. We compute this as an
// affine expression ceilDiv (upperBound - lowerBound) step. We use
diff --git a/mlir/test/Conversion/SCFToGPU/parallel_loop.mlir b/mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
index 2f192df1dad2e..ead8b9e12ee77 100644
--- a/mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
+++ b/mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
@@ -259,7 +259,8 @@ module {
// CHECK: [[VAL_11:%.*]] = affine.apply #[[$MAP1]]([[VAL_8]]){{\[}}%[[C0]], %[[C3]]]
// CHECK: [[VAL_12:%.*]] = arith.constant 4 : index
// CHECK: [[VAL_13:%.*]] = affine.apply #[[$MAP1]]([[VAL_12]]){{\[}}%[[C0]], %[[C1]]]
-// CHECK: [[VAL_15:%.*]] = affine.apply #[[$MAP1]](%[[C3]]){{\[}}%[[C0]], %[[C1]]]
+// CHECK: [[VAL_14:%.*]] = arith.constant 3 : index
+// CHECK: [[VAL_15:%.*]] = affine.apply #[[$MAP1]]([[VAL_14]]){{\[}}%[[C0]], %[[C1]]]
// CHECK: gpu.launch blocks([[VAL_16:%.*]], [[VAL_17:%.*]], [[VAL_18:%.*]]) in ([[VAL_19:%.*]] = [[VAL_10]], [[VAL_20:%.*]] = [[VAL_11]], [[VAL_21:%.*]] = [[VAL_9]]) threads([[VAL_22:%.*]], [[VAL_23:%.*]], [[VAL_24:%.*]]) in ([[VAL_25:%.*]] = [[VAL_13]], [[VAL_26:%.*]] = [[VAL_15]], [[VAL_27:%.*]] = [[VAL_9]]) {
// CHECK: [[VAL_28:%.*]] = affine.apply #[[$MAP2]]([[VAL_16]]){{\[}}%[[C2]], %[[C0]]]
// CHECK: [[VAL_29:%.*]] = affine.apply #[[$MAP2]]([[VAL_17]]){{\[}}%[[C3]], %[[C0]]]
@@ -721,3 +722,40 @@ func.func @scf2gpu_index_creation_1d() {
// CHECK: gpu.launch
// CHECK: %[[IDX:.*]] = affine.apply
// CHECK: arith.addi %[[IDX]],
+
+// -----
+
+// The upper bound of the inner loop cannot be derived to a constant: it is an
+// `arith.muli` whose operand is the outer induction variable. Deriving a bound
+// for that operand fails, so `arith.minsi` has to fall back to its other
+// operand instead of dereferencing the failed result.
+
+func.func @nested_underivable_upper_bound(%buf : memref<?xf32>) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ %c-1 = arith.constant -1 : index
+ %c64 = arith.constant 64 : index
+
+ scf.parallel (%i) = (%c0) to (%c64) step (%c1) {
+ %scaled = arith.muli %i, %c1 : index
+ %bound = arith.minsi %scaled, %c-1 : index
+ scf.parallel (%j) = (%c0) to (%bound) step (%c1) {
+ %v = memref.load %buf[%j] : memref<?xf32>
+ } {
+ mapping = [
+ #gpu.loop_dim_map<processor = thread_x, map = (d0) -> (d0), bound = (d0) -> (d0)>
+ ]
+ }
+ } {
+ mapping = [
+ #gpu.loop_dim_map<processor = block_x, map = (d0) -> (d0), bound = (d0) -> (d0)>
+ ]
+ }
+ return
+}
+
+// CHECK-LABEL: func @nested_underivable_upper_bound
+// CHECK: gpu.launch
+// CHECK: %[[BOUND:.*]] = arith.minsi
+// CHECK: %[[IV:.*]] = affine.apply
+// CHECK: arith.cmpi slt, %[[IV]], %[[BOUND]]
More information about the Mlir-commits
mailing list