[Mlir-commits] [mlir] [mlir][memref] Allow out-of-bounds semantics for memref.subview (PR #152164)

[Kunwar Grover]

https://github.com/Groverkss created https://github.com/llvm/llvm-project/pull/152164

out-of-bounds behavior for memref.subview was disallowed by https://github.com/llvm/llvm-project/pull/131876 , but it adds multiple problems to the problem definition:

1. The op definition introduces undefined behavior to the operation. This means the operation is not Pure anymore. This is already a bug:

```
0<= %arg0 < 128
if (%arg0 < 64) {
  %subview = memref.subview %mem[%arg0][64][1] : memref<64xf32>
  memref.store %arg1, %subview[0]
} 
```

Because memref.subview is undefined if the subview is hoisted out, this introduces UB when hoisted out, which it will because it is marked as Pure.

2. The op has ir that verifies, but will not verify after canonicalization:

```
%i = arith.constant 10 : index
memref.subview %mem[%i][10][1] : memref<10xf32>
```

This ir will verify, but after canonicalization it will not verify. This makes the operation definition inconsistent.

There are more issues with having this out of bounds verifier, but these two issues above are already enough to warrant reverting this behavior.

>From d91eb3eea498b6de13d8a000d807aa6191acca1f Mon Sep 17 00:00:00 2001
From: Kunwar Grover <groverkss at gmail.com>
Date: Tue, 5 Aug 2025 16:17:45 +0100
Subject: [PATCH] [mlir][memref] Allow out-of-bounds semantics for
 memref.subview

---
 .../include/mlir/Dialect/MemRef/IR/MemRefOps.td |  8 +++-----
 .../include/mlir/Interfaces/ViewLikeInterface.h | 17 ++++++++++-------
 mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp        | 16 ++++------------
 mlir/test/Dialect/MemRef/invalid.mlir           | 16 ----------------
 4 files changed, 17 insertions(+), 40 deletions(-)

diff --git a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
index 9321089ab55fa..3c34b7b9e00e9 100644
--- a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
@@ -2047,11 +2047,9 @@ def SubViewOp : MemRef_OpWithOffsetSizesAndStrides<"subview", [
     result_offset = src_offset + dot_product(offset_operands, src_strides)
     ```
 
-    The offset, size and stride operands must be in-bounds with respect to the
-    source memref. When possible, the static operation verifier will detect
-    out-of-bounds subviews. Subviews that cannot be confirmed to be in-bounds
-    or out-of-bounds based on compile-time information are valid. However,
-    performing an out-of-bounds subview at runtime is undefined behavior.
+    The operation does not guarantee if the created subview is in-bounds. It is
+    the responsibility of the user to guarantee there are no out-of-bounds
+    accesses into the subview.
 
     Example 1:
 
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.h b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
index db9c37fc3dc99..d175acb3dfd29 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
@@ -76,7 +76,8 @@ SliceBoundsVerificationResult verifyInBoundsSlice(
 /// returns the new result type of the op, based on the new offsets, sizes and
 /// strides. `CastOpFunc` is used to generate a cast op if the result type of
 /// the op has changed.
-template <typename OpType, typename ResultTypeFn, typename CastOpFunc>
+template <typename OpType, typename ResultTypeFn, typename CastOpFunc,
+          bool CheckInBounds = true>
 class OpWithOffsetSizesAndStridesConstantArgumentFolder final
     : public OpRewritePattern<OpType> {
 public:
@@ -94,12 +95,14 @@ class OpWithOffsetSizesAndStridesConstantArgumentFolder final
         failed(foldDynamicIndexList(mixedStrides)))
       return failure();
 
-    // Pattern does not apply if the produced op would not verify.
-    SliceBoundsVerificationResult sliceResult = verifyInBoundsSlice(
-        cast<ShapedType>(op.getSource().getType()).getShape(), mixedOffsets,
-        mixedSizes, mixedStrides);
-    if (!sliceResult.isValid)
-      return failure();
+    if (CheckInBounds) {
+      // Pattern does not apply if the produced op would not verify.
+      SliceBoundsVerificationResult sliceResult = verifyInBoundsSlice(
+          cast<ShapedType>(op.getSource().getType()).getShape(), mixedOffsets,
+          mixedSizes, mixedStrides);
+      if (!sliceResult.isValid)
+        return failure();
+    }
 
     // Compute the new result type.
     auto resultType =
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 74b968c27a62a..6e5ca3678ec59 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -2977,14 +2977,6 @@ LogicalResult SubViewOp::verify() {
     return produceSubViewErrorMsg(SliceVerificationResult::LayoutMismatch,
                                   *this, expectedType);
 
-  // Verify that offsets, sizes, strides do not run out-of-bounds with respect
-  // to the base memref.
-  SliceBoundsVerificationResult boundsResult =
-      verifyInBoundsSlice(baseType.getShape(), staticOffsets, staticSizes,
-                          staticStrides, /*generateErrorMessage=*/true);
-  if (!boundsResult.isValid)
-    return getOperation()->emitError(boundsResult.errorMessage);
-
   return success();
 }
 
@@ -3253,10 +3245,10 @@ struct SubViewCanonicalizer {
 
 void SubViewOp::getCanonicalizationPatterns(RewritePatternSet &results,
                                             MLIRContext *context) {
-  results
-      .add<OpWithOffsetSizesAndStridesConstantArgumentFolder<
-               SubViewOp, SubViewReturnTypeCanonicalizer, SubViewCanonicalizer>,
-           SubViewOpMemRefCastFolder, TrivialSubViewOpFolder>(context);
+  results.add<OpWithOffsetSizesAndStridesConstantArgumentFolder<
+                  SubViewOp, SubViewReturnTypeCanonicalizer,
+                  SubViewCanonicalizer, /*CheckInBounds=*/false>,
+              SubViewOpMemRefCastFolder, TrivialSubViewOpFolder>(context);
 }
 
 OpFoldResult SubViewOp::fold(FoldAdaptor adaptor) {
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir b/mlir/test/Dialect/MemRef/invalid.mlir
index b4476036d6513..e7a48979d11d0 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -745,22 +745,6 @@ func.func @invalid_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
 
 // -----
 
-func.func @invalid_subview(%arg0: memref<10xf32>) {
-  // expected-error at +1 {{offset 0 is out-of-bounds: 10 >= 10}}
-  %0 = memref.subview %arg0 [10][1][1] : memref<10xf32> to memref<1xf32, strided<[1], offset: 10>>
-  return
-}
-
-// -----
-
-func.func @invalid_subview(%arg0: memref<9xf32>) {
-  // expected-error at +1 {{slice along dimension 0 runs out-of-bounds: 9 >= 9}}
-  %0 = memref.subview %arg0 [3][4][2] : memref<9xf32> to memref<4xf32, strided<[2], offset: 3>>
-  return
-}
-
-// -----
-
 func.func @invalid_rank_reducing_subview(%arg0 : index, %arg1 : index, %arg2 : index) {
   %0 = memref.alloc() : memref<8x16x4xf32>
   // expected-error at +1 {{expected result type to be 'memref<8x16x4xf32, strided<[64, 4, 1]>>' or a rank-reduced version. (mismatch of result sizes)}}