[Mlir-commits] [mlir] [mlir][vector] Don't fold in_bounds for negative constant indices (PR #219681)

[Dhairyashil R G]

https://github.com/dhairyashilRG created https://github.com/llvm/llvm-project/pull/219681

`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

>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] [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