[Mlir-commits] [mlir] [mlir][vector] Fix VectorToSCF in-bounds check for negative indices (PR #224843)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon Sep 21 23:30:49 PDT 2026
https://github.com/StarOne01 updated https://github.com/llvm/llvm-project/pull/224843
>From 41782af03156a31cf2937cf9742dad90226118fb Mon Sep 17 00:00:00 2001
From: Prashanth <ping at starone01.me>
Date: Sat, 19 Sep 2026 21:49:45 +0530
Subject: [PATCH 1/3] [mlir][vector] Fix VectorToSCF in-bounds check for
negative indices
generateInBoundsCheck used a signed comparison (sgt) to guard the
runtime in-bounds branch. A negative dynamic index satisfies the
signed comparison against the upper bound and is incorrectly
treated as in-bounds, permitting an out-of-bounds access.
Switch to an unsigned comparison (ugt): a negative value
reinterpreted as unsigned becomes larger than any valid bound,
so the existing upper-bound check alone now also correctly
rejects negative indices.
Fixes #223258
---
.../Conversion/VectorToSCF/VectorToSCF.cpp | 2 +-
.../Conversion/VectorToSCF/vector-to-scf.mlir | 40 ++++++++++++++-----
2 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index b8eed6315e389..de63de1146d2a 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -201,7 +201,7 @@ static Value generateInBoundsCheck(
Value base = xferOp.getIndices()[*dim];
Value memrefIdx =
affine::makeComposedAffineApply(b, loc, d0 + d1, {base, iv});
- cond = arith::CmpIOp::create(lb, arith::CmpIPredicate::sgt, memrefDim,
+ cond = arith::CmpIOp::create(lb, arith::CmpIPredicate::ugt, memrefDim,
memrefIdx);
}
diff --git a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
index e3a7f57e144ed..788ed51050000 100644
--- a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
+++ b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
@@ -232,7 +232,7 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// CHECK: scf.for %[[I:.*]] = %[[C0]] to %[[C3]]
// CHECK: %[[dim:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
// CHECK: %[[add:.*]] = affine.apply #[[$MAP0]](%[[I]])[%[[base]]]
- // CHECK: %[[cond1:.*]] = arith.cmpi sgt, %[[dim]], %[[add]] : index
+ // CHECK: %[[cond1:.*]] = arith.cmpi ugt, %[[dim]], %[[add]] : index
// CHECK: scf.if %[[cond1]] {
// CHECK: %[[vec_1d:.*]] = vector.transfer_read %[[A]][%{{.*}}, %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// CHECK: memref.store %[[vec_1d]], %[[alloc_casted]][%[[I]]] : memref<3xvector<15xf32>>
@@ -246,7 +246,7 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// FULL-UNROLL-DAG: %[[VEC0:.*]] = arith.constant dense<7.000000e+00> : vector<3x15xf32>
// FULL-UNROLL-DAG: %[[C0:.*]] = arith.constant 0 : index
// FULL-UNROLL: %[[DIM:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
- // FULL-UNROLL: cmpi sgt, %[[DIM]], %[[base]] : index
+ // FULL-UNROLL: cmpi ugt, %[[DIM]], %[[base]] : index
// FULL-UNROLL: %[[VEC1:.*]] = scf.if %{{.*}} -> (vector<3x15xf32>) {
// FULL-UNROLL: vector.transfer_read %[[A]][%[[base]], %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// FULL-UNROLL: vector.insert %{{.*}}, %[[VEC0]] [0] : vector<15xf32> into vector<3x15xf32>
@@ -255,7 +255,7 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// FULL-UNROLL: scf.yield %{{.*}} : vector<3x15xf32>
// FULL-UNROLL: }
// FULL-UNROLL: affine.apply #[[$MAP1]]()[%[[base]]]
- // FULL-UNROLL: cmpi sgt, %{{.*}}, %{{.*}} : index
+ // FULL-UNROLL: cmpi ugt, %{{.*}}, %{{.*}} : index
// FULL-UNROLL: %[[VEC2:.*]] = scf.if %{{.*}} -> (vector<3x15xf32>) {
// FULL-UNROLL: vector.transfer_read %[[A]][%{{.*}}, %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// FULL-UNROLL: vector.insert %{{.*}}, %[[VEC1]] [1] : vector<15xf32> into vector<3x15xf32>
@@ -264,7 +264,7 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// FULL-UNROLL: scf.yield %{{.*}} : vector<3x15xf32>
// FULL-UNROLL: }
// FULL-UNROLL: affine.apply #[[$MAP2]]()[%[[base]]]
- // FULL-UNROLL: cmpi sgt, %{{.*}}, %{{.*}} : index
+ // FULL-UNROLL: cmpi ugt, %{{.*}}, %{{.*}} : index
// FULL-UNROLL: %[[VEC3:.*]] = scf.if %{{.*}} -> (vector<3x15xf32>) {
// FULL-UNROLL: vector.transfer_read %[[A]][%{{.*}}, %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// FULL-UNROLL: vector.insert %{{.*}}, %[[VEC2]] [2] : vector<15xf32> into vector<3x15xf32>
@@ -304,7 +304,7 @@ func.func @transfer_write_progressive(%A : memref<?x?xf32>, %base: index, %vec:
// CHECK: scf.for %[[I:.*]] = %[[C0]] to %[[C3]]
// CHECK: %[[dim:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
// CHECK: %[[add:.*]] = affine.apply #[[$MAP0]](%[[I]])[%[[base]]]
- // CHECK: %[[cmp:.*]] = arith.cmpi sgt, %[[dim]], %[[add]] : index
+ // CHECK: %[[cmp:.*]] = arith.cmpi ugt, %[[dim]], %[[add]] : index
// CHECK: scf.if %[[cmp]] {
// CHECK: %[[vec_1d:.*]] = memref.load %[[vmemref]][%[[I]]] : memref<3xvector<15xf32>>
// CHECK: vector.transfer_write %[[vec_1d]], %[[A]][{{.*}}, %[[base]]] : vector<15xf32>, memref<?x?xf32>
@@ -313,19 +313,19 @@ func.func @transfer_write_progressive(%A : memref<?x?xf32>, %base: index, %vec:
// FULL-UNROLL: %[[C0:.*]] = arith.constant 0 : index
// FULL-UNROLL: %[[DIM:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
- // FULL-UNROLL: %[[CMP0:.*]] = arith.cmpi sgt, %[[DIM]], %[[base]] : index
+ // FULL-UNROLL: %[[CMP0:.*]] = arith.cmpi ugt, %[[DIM]], %[[base]] : index
// FULL-UNROLL: scf.if %[[CMP0]] {
// FULL-UNROLL: %[[V0:.*]] = vector.extract %[[vec]][0] : vector<15xf32> from vector<3x15xf32>
// FULL-UNROLL: vector.transfer_write %[[V0]], %[[A]][%[[base]], %[[base]]] : vector<15xf32>, memref<?x?xf32>
// FULL-UNROLL: }
// FULL-UNROLL: %[[I1:.*]] = affine.apply #[[$MAP1]]()[%[[base]]]
- // FULL-UNROLL: %[[CMP1:.*]] = arith.cmpi sgt, %{{.*}}, %[[I1]] : index
+ // FULL-UNROLL: %[[CMP1:.*]] = arith.cmpi ugt, %{{.*}}, %[[I1]] : index
// FULL-UNROLL: scf.if %[[CMP1]] {
// FULL-UNROLL: %[[V1:.*]] = vector.extract %[[vec]][1] : vector<15xf32> from vector<3x15xf32>
// FULL-UNROLL: vector.transfer_write %[[V1]], %[[A]][%{{.*}}, %[[base]]] : vector<15xf32>, memref<?x?xf32>
// FULL-UNROLL: }
// FULL-UNROLL: %[[I2:.*]] = affine.apply #[[$MAP2]]()[%[[base]]]
- // FULL-UNROLL: %[[CMP2:.*]] = arith.cmpi sgt, %{{.*}}, %[[I2]] : index
+ // FULL-UNROLL: %[[CMP2:.*]] = arith.cmpi ugt, %{{.*}}, %[[I2]] : index
// FULL-UNROLL: scf.if %[[CMP2]] {
// FULL-UNROLL: %[[V2:.*]] = vector.extract %[[vec]][2] : vector<15xf32> from vector<3x15xf32>
// FULL-UNROLL: vector.transfer_write %[[V2]], %[[A]][%{{.*}}, %[[base]]] : vector<15xf32>, memref<?x?xf32>
@@ -414,7 +414,7 @@ func.func @transfer_read_minor_identity(%A : memref<?x?x?x?xf32>) -> vector<3x3x
// CHECK: %[[cast:.*]] = vector.type_cast %[[m]] : memref<vector<3x3xf32>> to memref<3xvector<3xf32>>
// CHECK: scf.for %[[arg1:.*]] = %[[c0]] to %[[c3]]
// CHECK: %[[d:.*]] = memref.dim %[[A]], %[[c2]] : memref<?x?x?x?xf32>
-// CHECK: %[[cmp:.*]] = arith.cmpi sgt, %[[d]], %[[arg1]] : index
+// CHECK: %[[cmp:.*]] = arith.cmpi ugt, %[[d]], %[[arg1]] : index
// CHECK: scf.if %[[cmp]] {
// CHECK: %[[tr:.*]] = vector.transfer_read %[[A]][%c0, %c0, %[[arg1]], %c0], %[[f0]] : memref<?x?x?x?xf32>, vector<3xf32>
// CHECK: memref.store %[[tr]], %[[cast]][%[[arg1]]] : memref<3xvector<3xf32>>
@@ -446,7 +446,7 @@ func.func @transfer_write_minor_identity(%A : vector<3x3xf32>, %B : memref<?x?x?
// CHECK: %[[cast:.*]] = vector.type_cast %[[m]] : memref<vector<3x3xf32>> to memref<3xvector<3xf32>>
// CHECK: scf.for %[[arg2:.*]] = %[[c0]] to %[[c3]]
// CHECK: %[[d:.*]] = memref.dim %[[B]], %[[c2]] : memref<?x?x?x?xf32>
-// CHECK: %[[cmp:.*]] = arith.cmpi sgt, %[[d]], %[[arg2]] : index
+// CHECK: %[[cmp:.*]] = arith.cmpi ugt, %[[d]], %[[arg2]] : index
// CHECK: scf.if %[[cmp]] {
// CHECK: %[[tmp:.*]] = memref.load %[[cast]][%[[arg2]]] : memref<3xvector<3xf32>>
// CHECK: vector.transfer_write %[[tmp]], %[[B]][%[[c0]], %[[c0]], %[[arg2]], %[[c0]]] : vector<3xf32>, memref<?x?x?x?xf32>
@@ -923,3 +923,23 @@ func.func private @transfer_write_no_alloc_scope()
%cst = arith.constant dense<0.0> : vector<2x3xf32>
%m = memref.alloc() : memref<2x3xf32>
vector.transfer_write %cst, %m[%c0, %c0] : vector<2x3xf32>, memref<2x3xf32>
+
+// -----
+
+// Negative test: dynamic start index. The upper-bound comparison alone
+// (`%idx < %dimSize`) would incorrectly treat a negative %idx as in-bounds
+// since it also satisfies a signed less-than against a positive bound. The
+// guard must use an unsigned comparison so that a negative index, reinterpreted
+// as unsigned, is correctly seen as out-of-range.
+// CHECK-LABEL: func.func @transfer_read_neg_start_guard
+func.func @transfer_read_neg_start_guard(%a: memref<4x8xf32>, %i: index)
+ -> vector<2x8xf32> {
+ %pad = arith.constant -42.0 : f32
+ // Static dim folds `ugt %c4, %idx` to `ult %idx, %c4` (unsigned, so idx < 0 is OOB).
+ // CHECK: arith.cmpi ult
+ // CHECK-NOT: arith.cmpi slt
+ // CHECK-NOT: arith.cmpi ugt
+ %v = vector.transfer_read %a[%i, %i], %pad {in_bounds = [false, true]}
+ : memref<4x8xf32>, vector<2x8xf32>
+ return %v : vector<2x8xf32>
+}
>From 40e3042b466db33f7b5a55ed39b84931c509437a Mon Sep 17 00:00:00 2001
From: Prashanth <ping at starone01.me>
Date: Tue, 22 Sep 2026 11:50:39 +0530
Subject: [PATCH 2/3] [mlir][VectorToSCF] Use explicit dual bound checks
instead of unsigned trick for readability
In commit 41782af, runtime in-bounds checking was updated from `sgt` to
`ugt` to reject negative dynamic indices using unsigned reinterpretation.
While `ugt` is functionally equivalent assuming non-negative memref
dimensions (guaranteed by MLIR's `ShapedType` interface), relying on an
unsigned reinterpretation trick reduces code readability and makes the
generated IR harder to audit.
Per review feedback, replace the single `ugt` check with explicit dual
signed checks:
1. Lower-bound check: `arith.cmpi sge, %idx, %c0`
2. Upper-bound check: `arith.cmpi slt, %idx, %dim`
3. Combine both with `arith.andi`
Update test assertions across loop and unrolled transfer lowerings, and
add a test verifying that explicit checks are emitted instead of `ugt`/`sgt`.
---
.../Conversion/VectorToSCF/VectorToSCF.cpp | 8 ++-
.../Conversion/VectorToSCF/vector-to-scf.mlir | 61 +++++++++++++------
2 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index de63de1146d2a..4497b24497512 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -201,8 +201,12 @@ static Value generateInBoundsCheck(
Value base = xferOp.getIndices()[*dim];
Value memrefIdx =
affine::makeComposedAffineApply(b, loc, d0 + d1, {base, iv});
- cond = arith::CmpIOp::create(lb, arith::CmpIPredicate::ugt, memrefDim,
- memrefIdx);
+ Value zero = arith::ConstantIndexOp::create(lb, 0);
+ Value nonNegative =
+ arith::CmpIOp::create(lb, arith::CmpIPredicate::sge, memrefIdx, zero);
+ Value inRange = arith::CmpIOp::create(lb, arith::CmpIPredicate::slt,
+ memrefIdx, memrefDim);
+ cond = arith::AndIOp::create(lb, nonNegative, inRange);
}
// Condition check 2: Masked in?
diff --git a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
index 788ed51050000..61e8b3b1f24b0 100644
--- a/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
+++ b/mlir/test/Conversion/VectorToSCF/vector-to-scf.mlir
@@ -232,7 +232,9 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// CHECK: scf.for %[[I:.*]] = %[[C0]] to %[[C3]]
// CHECK: %[[dim:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
// CHECK: %[[add:.*]] = affine.apply #[[$MAP0]](%[[I]])[%[[base]]]
- // CHECK: %[[cond1:.*]] = arith.cmpi ugt, %[[dim]], %[[add]] : index
+ // CHECK: %[[nonneg:.*]] = arith.cmpi sge, %[[add]], %[[C0]] : index
+ // CHECK: %[[inrange:.*]] = arith.cmpi slt, %[[add]], %[[dim]] : index
+ // CHECK: %[[cond1:.*]] = arith.andi %[[nonneg]], %[[inrange]] : i1
// CHECK: scf.if %[[cond1]] {
// CHECK: %[[vec_1d:.*]] = vector.transfer_read %[[A]][%{{.*}}, %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// CHECK: memref.store %[[vec_1d]], %[[alloc_casted]][%[[I]]] : memref<3xvector<15xf32>>
@@ -246,7 +248,9 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// FULL-UNROLL-DAG: %[[VEC0:.*]] = arith.constant dense<7.000000e+00> : vector<3x15xf32>
// FULL-UNROLL-DAG: %[[C0:.*]] = arith.constant 0 : index
// FULL-UNROLL: %[[DIM:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
- // FULL-UNROLL: cmpi ugt, %[[DIM]], %[[base]] : index
+ // FULL-UNROLL: %[[NN0:.*]] = arith.cmpi sge, %[[base]], %[[C0]] : index
+ // FULL-UNROLL: %[[IR0:.*]] = arith.cmpi slt, %[[base]], %[[DIM]] : index
+ // FULL-UNROLL: %{{.*}} = arith.andi %[[NN0]], %[[IR0]] : i1
// FULL-UNROLL: %[[VEC1:.*]] = scf.if %{{.*}} -> (vector<3x15xf32>) {
// FULL-UNROLL: vector.transfer_read %[[A]][%[[base]], %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// FULL-UNROLL: vector.insert %{{.*}}, %[[VEC0]] [0] : vector<15xf32> into vector<3x15xf32>
@@ -254,8 +258,10 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// FULL-UNROLL: } else {
// FULL-UNROLL: scf.yield %{{.*}} : vector<3x15xf32>
// FULL-UNROLL: }
- // FULL-UNROLL: affine.apply #[[$MAP1]]()[%[[base]]]
- // FULL-UNROLL: cmpi ugt, %{{.*}}, %{{.*}} : index
+ // FULL-UNROLL: %[[I1:.*]] = affine.apply #[[$MAP1]]()[%[[base]]]
+ // FULL-UNROLL: %[[NN1:.*]] = arith.cmpi sge, %[[I1]], %[[C0]] : index
+ // FULL-UNROLL: %[[IR1:.*]] = arith.cmpi slt, %[[I1]], %{{.*}} : index
+ // FULL-UNROLL: %{{.*}} = arith.andi %[[NN1]], %[[IR1]] : i1
// FULL-UNROLL: %[[VEC2:.*]] = scf.if %{{.*}} -> (vector<3x15xf32>) {
// FULL-UNROLL: vector.transfer_read %[[A]][%{{.*}}, %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// FULL-UNROLL: vector.insert %{{.*}}, %[[VEC1]] [1] : vector<15xf32> into vector<3x15xf32>
@@ -263,8 +269,10 @@ func.func @transfer_read_progressive(%A : memref<?x?xf32>, %base: index) -> vect
// FULL-UNROLL: } else {
// FULL-UNROLL: scf.yield %{{.*}} : vector<3x15xf32>
// FULL-UNROLL: }
- // FULL-UNROLL: affine.apply #[[$MAP2]]()[%[[base]]]
- // FULL-UNROLL: cmpi ugt, %{{.*}}, %{{.*}} : index
+ // FULL-UNROLL: %[[I2:.*]] = affine.apply #[[$MAP2]]()[%[[base]]]
+ // FULL-UNROLL: %[[NN2:.*]] = arith.cmpi sge, %[[I2]], %[[C0]] : index
+ // FULL-UNROLL: %[[IR2:.*]] = arith.cmpi slt, %[[I2]], %{{.*}} : index
+ // FULL-UNROLL: %{{.*}} = arith.andi %[[NN2]], %[[IR2]] : i1
// FULL-UNROLL: %[[VEC3:.*]] = scf.if %{{.*}} -> (vector<3x15xf32>) {
// FULL-UNROLL: vector.transfer_read %[[A]][%{{.*}}, %[[base]]], %[[C7]] : memref<?x?xf32>, vector<15xf32>
// FULL-UNROLL: vector.insert %{{.*}}, %[[VEC2]] [2] : vector<15xf32> into vector<3x15xf32>
@@ -304,7 +312,9 @@ func.func @transfer_write_progressive(%A : memref<?x?xf32>, %base: index, %vec:
// CHECK: scf.for %[[I:.*]] = %[[C0]] to %[[C3]]
// CHECK: %[[dim:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
// CHECK: %[[add:.*]] = affine.apply #[[$MAP0]](%[[I]])[%[[base]]]
- // CHECK: %[[cmp:.*]] = arith.cmpi ugt, %[[dim]], %[[add]] : index
+ // CHECK: %[[nonneg:.*]] = arith.cmpi sge, %[[add]], %[[C0]] : index
+ // CHECK: %[[inrange:.*]] = arith.cmpi slt, %[[add]], %[[dim]] : index
+ // CHECK: %[[cmp:.*]] = arith.andi %[[nonneg]], %[[inrange]] : i1
// CHECK: scf.if %[[cmp]] {
// CHECK: %[[vec_1d:.*]] = memref.load %[[vmemref]][%[[I]]] : memref<3xvector<15xf32>>
// CHECK: vector.transfer_write %[[vec_1d]], %[[A]][{{.*}}, %[[base]]] : vector<15xf32>, memref<?x?xf32>
@@ -313,19 +323,25 @@ func.func @transfer_write_progressive(%A : memref<?x?xf32>, %base: index, %vec:
// FULL-UNROLL: %[[C0:.*]] = arith.constant 0 : index
// FULL-UNROLL: %[[DIM:.*]] = memref.dim %[[A]], %[[C0]] : memref<?x?xf32>
- // FULL-UNROLL: %[[CMP0:.*]] = arith.cmpi ugt, %[[DIM]], %[[base]] : index
+ // FULL-UNROLL: %[[NN0:.*]] = arith.cmpi sge, %[[base]], %[[C0]] : index
+ // FULL-UNROLL: %[[IR0:.*]] = arith.cmpi slt, %[[base]], %[[DIM]] : index
+ // FULL-UNROLL: %[[CMP0:.*]] = arith.andi %[[NN0]], %[[IR0]] : i1
// FULL-UNROLL: scf.if %[[CMP0]] {
// FULL-UNROLL: %[[V0:.*]] = vector.extract %[[vec]][0] : vector<15xf32> from vector<3x15xf32>
// FULL-UNROLL: vector.transfer_write %[[V0]], %[[A]][%[[base]], %[[base]]] : vector<15xf32>, memref<?x?xf32>
// FULL-UNROLL: }
// FULL-UNROLL: %[[I1:.*]] = affine.apply #[[$MAP1]]()[%[[base]]]
- // FULL-UNROLL: %[[CMP1:.*]] = arith.cmpi ugt, %{{.*}}, %[[I1]] : index
+ // FULL-UNROLL: %[[NN1:.*]] = arith.cmpi sge, %[[I1]], %[[C0]] : index
+ // FULL-UNROLL: %[[IR1:.*]] = arith.cmpi slt, %[[I1]], %{{.*}} : index
+ // FULL-UNROLL: %[[CMP1:.*]] = arith.andi %[[NN1]], %[[IR1]] : i1
// FULL-UNROLL: scf.if %[[CMP1]] {
// FULL-UNROLL: %[[V1:.*]] = vector.extract %[[vec]][1] : vector<15xf32> from vector<3x15xf32>
// FULL-UNROLL: vector.transfer_write %[[V1]], %[[A]][%{{.*}}, %[[base]]] : vector<15xf32>, memref<?x?xf32>
// FULL-UNROLL: }
// FULL-UNROLL: %[[I2:.*]] = affine.apply #[[$MAP2]]()[%[[base]]]
- // FULL-UNROLL: %[[CMP2:.*]] = arith.cmpi ugt, %{{.*}}, %[[I2]] : index
+ // FULL-UNROLL: %[[NN2:.*]] = arith.cmpi sge, %[[I2]], %[[C0]] : index
+ // FULL-UNROLL: %[[IR2:.*]] = arith.cmpi slt, %[[I2]], %{{.*}} : index
+ // FULL-UNROLL: %[[CMP2:.*]] = arith.andi %[[NN2]], %[[IR2]] : i1
// FULL-UNROLL: scf.if %[[CMP2]] {
// FULL-UNROLL: %[[V2:.*]] = vector.extract %[[vec]][2] : vector<15xf32> from vector<3x15xf32>
// FULL-UNROLL: vector.transfer_write %[[V2]], %[[A]][%{{.*}}, %[[base]]] : vector<15xf32>, memref<?x?xf32>
@@ -414,7 +430,9 @@ func.func @transfer_read_minor_identity(%A : memref<?x?x?x?xf32>) -> vector<3x3x
// CHECK: %[[cast:.*]] = vector.type_cast %[[m]] : memref<vector<3x3xf32>> to memref<3xvector<3xf32>>
// CHECK: scf.for %[[arg1:.*]] = %[[c0]] to %[[c3]]
// CHECK: %[[d:.*]] = memref.dim %[[A]], %[[c2]] : memref<?x?x?x?xf32>
-// CHECK: %[[cmp:.*]] = arith.cmpi ugt, %[[d]], %[[arg1]] : index
+// CHECK: %[[nonneg:.*]] = arith.cmpi sge, %[[arg1]], %[[c0]] : index
+// CHECK: %[[inrange:.*]] = arith.cmpi slt, %[[arg1]], %[[d]] : index
+// CHECK: %[[cmp:.*]] = arith.andi %[[nonneg]], %[[inrange]] : i1
// CHECK: scf.if %[[cmp]] {
// CHECK: %[[tr:.*]] = vector.transfer_read %[[A]][%c0, %c0, %[[arg1]], %c0], %[[f0]] : memref<?x?x?x?xf32>, vector<3xf32>
// CHECK: memref.store %[[tr]], %[[cast]][%[[arg1]]] : memref<3xvector<3xf32>>
@@ -446,7 +464,9 @@ func.func @transfer_write_minor_identity(%A : vector<3x3xf32>, %B : memref<?x?x?
// CHECK: %[[cast:.*]] = vector.type_cast %[[m]] : memref<vector<3x3xf32>> to memref<3xvector<3xf32>>
// CHECK: scf.for %[[arg2:.*]] = %[[c0]] to %[[c3]]
// CHECK: %[[d:.*]] = memref.dim %[[B]], %[[c2]] : memref<?x?x?x?xf32>
-// CHECK: %[[cmp:.*]] = arith.cmpi ugt, %[[d]], %[[arg2]] : index
+// CHECK: %[[nonneg:.*]] = arith.cmpi sge, %[[arg2]], %[[c0]] : index
+// CHECK: %[[inrange:.*]] = arith.cmpi slt, %[[arg2]], %[[d]] : index
+// CHECK: %[[cmp:.*]] = arith.andi %[[nonneg]], %[[inrange]] : i1
// CHECK: scf.if %[[cmp]] {
// CHECK: %[[tmp:.*]] = memref.load %[[cast]][%[[arg2]]] : memref<3xvector<3xf32>>
// CHECK: vector.transfer_write %[[tmp]], %[[B]][%[[c0]], %[[c0]], %[[arg2]], %[[c0]]] : vector<3xf32>, memref<?x?x?x?xf32>
@@ -926,19 +946,20 @@ vector.transfer_write %cst, %m[%c0, %c0] : vector<2x3xf32>, memref<2x3xf32>
// -----
-// Negative test: dynamic start index. The upper-bound comparison alone
-// (`%idx < %dimSize`) would incorrectly treat a negative %idx as in-bounds
-// since it also satisfies a signed less-than against a positive bound. The
-// guard must use an unsigned comparison so that a negative index, reinterpreted
-// as unsigned, is correctly seen as out-of-range.
+// Negative test: dynamic start index. Access is only valid when
+// 0 <= memrefIdx < memrefDim, so the guard must check both bounds
+// explicitly rather than relying on an unsigned reinterpretation trick.
+
// CHECK-LABEL: func.func @transfer_read_neg_start_guard
func.func @transfer_read_neg_start_guard(%a: memref<4x8xf32>, %i: index)
-> vector<2x8xf32> {
%pad = arith.constant -42.0 : f32
- // Static dim folds `ugt %c4, %idx` to `ult %idx, %c4` (unsigned, so idx < 0 is OOB).
- // CHECK: arith.cmpi ult
- // CHECK-NOT: arith.cmpi slt
+ // CHECK: %[[zero:.*]] = arith.constant 0 : index
+ // CHECK: %[[nonneg:.*]] = arith.cmpi sge, %{{.*}}, %[[zero]] : index
+ // CHECK: %[[inrange:.*]] = arith.cmpi slt, %{{.*}}, %{{.*}} : index
+ // CHECK: %{{.*}} = arith.andi %[[nonneg]], %[[inrange]] : i1
// CHECK-NOT: arith.cmpi ugt
+ // CHECK-NOT: arith.cmpi sgt
%v = vector.transfer_read %a[%i, %i], %pad {in_bounds = [false, true]}
: memref<4x8xf32>, vector<2x8xf32>
return %v : vector<2x8xf32>
>From e7bc051113b68a5623dc6ed427041bb04a5546a2 Mon Sep 17 00:00:00 2001
From: Prashanth <ping at starone01.me>
Date: Tue, 22 Sep 2026 12:00:27 +0530
Subject: [PATCH 3/3] [mlir][Vector] Fix formatting of in-bounds check for
vector transfer operations
---
mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
index 4497b24497512..1a1927f494476 100644
--- a/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
+++ b/mlir/lib/Conversion/VectorToSCF/VectorToSCF.cpp
@@ -205,7 +205,7 @@ static Value generateInBoundsCheck(
Value nonNegative =
arith::CmpIOp::create(lb, arith::CmpIPredicate::sge, memrefIdx, zero);
Value inRange = arith::CmpIOp::create(lb, arith::CmpIPredicate::slt,
- memrefIdx, memrefDim);
+ memrefIdx, memrefDim);
cond = arith::AndIOp::create(lb, nonNegative, inRange);
}
More information about the Mlir-commits
mailing list