[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:21:04 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/2] [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/2] [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>



More information about the Mlir-commits mailing list