[Mlir-commits] [mlir] [mlir][vector] Don't fold in_bounds for negative constant indices (PR #219681)
Dhairyashil R G
llvmlistbot at llvm.org
Sat Sep 5 05:59:06 PDT 2026
https://github.com/dhairyashilRG updated https://github.com/llvm/llvm-project/pull/219681
>From 31051fa7d846fa52e9c9519fd3852172acf3b755 Mon Sep 17 00:00:00 2001
From: Dhairyashil R G <dhairyashil25 at gmail.com>
Date: Fri, 28 Aug 2026 21:48:29 +0530
Subject: [PATCH 1/3] [mlir][vector] Don't fold in_bounds for negative constant
indices
`isInBounds` checked only that the transfer *ends* inside the source:
return cstOp.value() + vectorSize <= sourceSize;
The `in_bounds` attribute promises more than that. Its definition in
VectorOps.td says accesses "(including the starting point)" may run
out-of-bounds when it is "false", so setting it to "true" is a claim about
the start of the transfer as well as its end.
With a negative constant index the two disagree. For
vector.transfer_read %m[-1] : memref<8xf32>, vector<4xf32>
`-1 + 4 <= 8` holds, so the fold set `in_bounds = [true]`, even though
element -1 is read from outside `%m`.
The same expression also overflows for a large enough index: at
`index = 2^63 - 1` the addition wraps and the fold again reports
`in_bounds = [true]`. Computing the bound as `sourceSize - vectorSize`
instead avoids the addition entirely; both operands are static, non-negative
dimension sizes, so the subtraction cannot overflow.
This is a spec-conformance fix, not a live miscompile report. A negative
index into a memref is already undefined behaviour, and the masked lowering
would not have caught it either: the guard `generateInBoundsCheck` emits in
VectorToSCF is `memrefDim > index + iv`, an upper-bound check only.
Adds three regression tests, each of which fails without the change.
Assisted-by: Claude
---
mlir/lib/Dialect/Vector/IR/VectorOps.cpp | 7 +++-
mlir/test/Dialect/Vector/canonicalize.mlir | 47 ++++++++++++++++++++++
2 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index f8f3deb2e4789..88de4ac0e0c0b 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -5396,8 +5396,13 @@ static bool isInBounds(TransferOp op, int64_t resultIdx, int64_t indicesIdx) {
int64_t sourceSize = op.getShapedType().getDimSize(indicesIdx);
int64_t vectorSize = op.getVectorType().getDimSize(resultIdx);
+ // Largest index at which a full vector still fits. Computed as a subtraction
+ // rather than adding to the index, which could overflow.
+ int64_t maxStart = sourceSize - vectorSize;
- return cstOp.value() + vectorSize <= sourceSize;
+ // `in_bounds` guarantees that the transfer stays within the source *including
+ // its starting point*, so a negative index is not in bounds.
+ return *cstOp >= 0 && *cstOp <= maxStart;
}
template <typename TransferOp>
diff --git a/mlir/test/Dialect/Vector/canonicalize.mlir b/mlir/test/Dialect/Vector/canonicalize.mlir
index 7ad6eda6ec1ba..5420d9a017530 100644
--- a/mlir/test/Dialect/Vector/canonicalize.mlir
+++ b/mlir/test/Dialect/Vector/canonicalize.mlir
@@ -1435,6 +1435,53 @@ func.func @no_fold_transfer_write_in_bounds_scalable(%m: memref<4xf32>, %v: vect
// -----
+// `in_bounds` promises that the transfer stays within the source *including its
+// starting point*, so a negative index is not in bounds even when the end of
+// the transfer would land inside the source. Here `-1 + 4 <= 8` holds, but
+// element `-1` is still read from outside `%m`.
+
+// CHECK-LABEL: func @no_fold_transfer_read_in_bounds_negative_index
+// CHECK: vector.transfer_read
+// CHECK-NOT: in_bounds
+// CHECK: : memref<8xf32>, vector<4xf32>
+func.func @no_fold_transfer_read_in_bounds_negative_index(%m: memref<8xf32>, %p: f32) -> vector<4xf32> {
+ %c-1 = arith.constant -1 : index
+ %v = vector.transfer_read %m[%c-1], %p : memref<8xf32>, vector<4xf32>
+ return %v : vector<4xf32>
+}
+
+// -----
+
+// Same for the write path, where the fold would permit a store to element -1.
+
+// CHECK-LABEL: func @no_fold_transfer_write_in_bounds_negative_index
+// CHECK: vector.transfer_write
+// CHECK-NOT: in_bounds
+// CHECK: : vector<4xf32>, memref<8xf32>
+func.func @no_fold_transfer_write_in_bounds_negative_index(%m: memref<8xf32>, %v: vector<4xf32>) {
+ %c-1 = arith.constant -1 : index
+ vector.transfer_write %v, %m[%c-1] : vector<4xf32>, memref<8xf32>
+ return
+}
+
+// -----
+
+// The bound is computed as `sourceSize - vectorSize` rather than
+// `index + vectorSize`, so an index large enough to overflow the addition is
+// still rejected.
+
+// CHECK-LABEL: func @no_fold_transfer_read_in_bounds_huge_index
+// CHECK: vector.transfer_read
+// CHECK-NOT: in_bounds
+// CHECK: : memref<8xf32>, vector<4xf32>
+func.func @no_fold_transfer_read_in_bounds_huge_index(%m: memref<8xf32>, %p: f32) -> vector<4xf32> {
+ %c = arith.constant 9223372036854775807 : index
+ %v = vector.transfer_read %m[%c], %p : memref<8xf32>, vector<4xf32>
+ return %v : vector<4xf32>
+}
+
+// -----
+
// CHECK-LABEL: fold_vector_transfers
func.func @fold_vector_transfers(%A: memref<?x8xf32>) -> (vector<4x8xf32>, vector<4x9xf32>) {
%c0 = arith.constant 0 : index
>From 90f032ea1c8921c406b6caca3f605d3737f9a8ef Mon Sep 17 00:00:00 2001
From: Dhairyashil Ghatage <dhairyashil25 at gmail.com>
Date: Tue, 1 Sep 2026 00:37:43 +0530
Subject: [PATCH 2/3] Address review: note why the subtraction cannot overflow
`maxStart = sourceSize - vectorSize` is only safe because the
`isDynamicDim` check earlier in `isInBounds` returns before reaching it.
`ShapedType::kDynamic` is `INT64_MIN`, so the subtraction would be signed
overflow on a dynamic dimension, and nothing at the subtraction connected
it to the guard that rules that out.
Comment only, no functional change.
Assisted-by: Claude
---
mlir/lib/Dialect/Vector/IR/VectorOps.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index 88de4ac0e0c0b..136b2f3d76679 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -5397,7 +5397,10 @@ static bool isInBounds(TransferOp op, int64_t resultIdx, int64_t indicesIdx) {
int64_t sourceSize = op.getShapedType().getDimSize(indicesIdx);
int64_t vectorSize = op.getVectorType().getDimSize(resultIdx);
// Largest index at which a full vector still fits. Computed as a subtraction
- // rather than adding to the index, which could overflow.
+ // rather than adding to the index, which could overflow. The subtraction is
+ // safe only because of the `isDynamicDim` early return above: `sourceSize`
+ // is a real static extent here, never `ShapedType::kDynamic`, which is
+ // `INT64_MIN` and would make this signed overflow.
int64_t maxStart = sourceSize - vectorSize;
// `in_bounds` guarantees that the transfer stays within the source *including
>From 4bdf38cbff82338f85ec489a5d5f934609fa83b9 Mon Sep 17 00:00:00 2001
From: Dhairyashil Ghatage <dhairyashil25 at gmail.com>
Date: Sat, 5 Sep 2026 18:24:50 +0530
Subject: [PATCH 3/3] Address review: trim test comments
@FedericoBruzzone noted the comments above the three new tests restate what
the test names already say. Drop the write-path comment, drop the overflow
comment and fold "overflow" into the test name instead, and cut the
negative-index comment down to the one fact the name cannot carry: the
transfer ends inside the source, so `-1 + 4 <= 8` holds and the fold looks
correct until you account for the starting point.
Tests only, no functional change.
Assisted-by: Claude
---
mlir/test/Dialect/Vector/canonicalize.mlir | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/mlir/test/Dialect/Vector/canonicalize.mlir b/mlir/test/Dialect/Vector/canonicalize.mlir
index 5420d9a017530..0314a38868567 100644
--- a/mlir/test/Dialect/Vector/canonicalize.mlir
+++ b/mlir/test/Dialect/Vector/canonicalize.mlir
@@ -1435,10 +1435,8 @@ func.func @no_fold_transfer_write_in_bounds_scalable(%m: memref<4xf32>, %v: vect
// -----
-// `in_bounds` promises that the transfer stays within the source *including its
-// starting point*, so a negative index is not in bounds even when the end of
-// the transfer would land inside the source. Here `-1 + 4 <= 8` holds, but
-// element `-1` is still read from outside `%m`.
+// `-1 + 4 <= 8` holds, but `in_bounds` covers the starting point too, so this
+// must not fold.
// CHECK-LABEL: func @no_fold_transfer_read_in_bounds_negative_index
// CHECK: vector.transfer_read
@@ -1452,8 +1450,6 @@ func.func @no_fold_transfer_read_in_bounds_negative_index(%m: memref<8xf32>, %p:
// -----
-// Same for the write path, where the fold would permit a store to element -1.
-
// CHECK-LABEL: func @no_fold_transfer_write_in_bounds_negative_index
// CHECK: vector.transfer_write
// CHECK-NOT: in_bounds
@@ -1466,15 +1462,11 @@ func.func @no_fold_transfer_write_in_bounds_negative_index(%m: memref<8xf32>, %v
// -----
-// The bound is computed as `sourceSize - vectorSize` rather than
-// `index + vectorSize`, so an index large enough to overflow the addition is
-// still rejected.
-
-// CHECK-LABEL: func @no_fold_transfer_read_in_bounds_huge_index
+// CHECK-LABEL: func @no_fold_transfer_read_in_bounds_index_overflow
// CHECK: vector.transfer_read
// CHECK-NOT: in_bounds
// CHECK: : memref<8xf32>, vector<4xf32>
-func.func @no_fold_transfer_read_in_bounds_huge_index(%m: memref<8xf32>, %p: f32) -> vector<4xf32> {
+func.func @no_fold_transfer_read_in_bounds_index_overflow(%m: memref<8xf32>, %p: f32) -> vector<4xf32> {
%c = arith.constant 9223372036854775807 : index
%v = vector.transfer_read %m[%c], %p : memref<8xf32>, vector<4xf32>
return %v : vector<4xf32>
More information about the Mlir-commits
mailing list