Skip to content

Conversation

@CoTinker
Copy link
Contributor

@CoTinker CoTinker commented Feb 11, 2025

This PR fixes an out-of-bounds bug that occurs when there are no overlap dimensions between the sizes and source of vector.extract_strided_slice, causing access to sizes to go out of bounds. Fixes #126196.

@llvmbot
Copy link
Member

llvmbot commented Feb 11, 2025

@llvm/pr-subscribers-mlir-vector

Author: Longsheng Mou (CoTinker)

Changes

This PR fixes an out-of-bounds bug that occurs when there are no unit dimensions in the source of vector.extract_strided_slice, causing access to sizes to go out of bounds. Fixes #126196.


Full diff: https://github.com/llvm/llvm-project/pull/126734.diff

2 Files Affected:

  • (modified) mlir/lib/Dialect/Vector/IR/VectorOps.cpp (+2-2)
  • (modified) mlir/test/Dialect/Vector/canonicalize.mlir (+11)
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index 30ff2df7c38fc3..f3f20b46add783 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -3968,8 +3968,8 @@ class ContiguousExtractStridedSliceToExtract final
     // Avoid generating slices that have leading unit dimensions. The shape_cast
     // op that we create below would take bad generic fallback patterns
     // (ShapeCastOpRewritePattern).
-    while (sizes[numOffsets] == 1 &&
-           numOffsets < static_cast<int>(sizes.size()) - 1) {
+    while (numOffsets < static_cast<int>(sizes.size()) - 1 &&
+           sizes[numOffsets] == 1) {
       ++numOffsets;
     }
 
diff --git a/mlir/test/Dialect/Vector/canonicalize.mlir b/mlir/test/Dialect/Vector/canonicalize.mlir
index 61e858f5f226a1..0fc0140666696f 100644
--- a/mlir/test/Dialect/Vector/canonicalize.mlir
+++ b/mlir/test/Dialect/Vector/canonicalize.mlir
@@ -2932,6 +2932,17 @@ func.func @contiguous_extract_strided_slices_to_extract(%arg0 : vector<8x1x2x1x1
 
 // -----
 
+// CHECK-LABEL: @contiguous_extract_strided_slices_to_extract_no_unit_dims
+// CHECK:        %[[EXTRACT:.+]] = vector.extract {{.*}}[0, 0] : vector<4xi32> from vector<8x2x4xi32>
+// CHECK-NEXT:   return %[[EXTRACT]] :  vector<4xi32>
+func.func @contiguous_extract_strided_slices_to_extract_no_unit_dims(%arg0 : vector<8x2x4xi32>) -> vector<4xi32> {
+  %1 = vector.extract_strided_slice %arg0 {offsets = [0, 0], sizes = [1, 1], strides = [1, 1]} : vector<8x2x4xi32> to vector<1x1x4xi32>
+  %2 = vector.shape_cast %1 : vector<1x1x4xi32> to vector<4xi32>
+  return %2 : vector<4xi32>
+}
+
+// -----
+
 // CHECK-LABEL: @contiguous_extract_strided_slices_to_extract_shorter_size_list
 // CHECK:        %[[EXTRACT:.+]] = vector.extract {{.*}}[0, 0, 0, 0] : vector<1x4xi32> from vector<8x1x2x1x1x4xi32>
 // CHECK-NEXT:   return %[[EXTRACT]] :  vector<1x4xi32>

@llvmbot
Copy link
Member

llvmbot commented Feb 11, 2025

@llvm/pr-subscribers-mlir

Author: Longsheng Mou (CoTinker)

Changes

This PR fixes an out-of-bounds bug that occurs when there are no unit dimensions in the source of vector.extract_strided_slice, causing access to sizes to go out of bounds. Fixes #126196.


Full diff: https://github.com/llvm/llvm-project/pull/126734.diff

2 Files Affected:

  • (modified) mlir/lib/Dialect/Vector/IR/VectorOps.cpp (+2-2)
  • (modified) mlir/test/Dialect/Vector/canonicalize.mlir (+11)
diff --git a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
index 30ff2df7c38fc3..f3f20b46add783 100644
--- a/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
+++ b/mlir/lib/Dialect/Vector/IR/VectorOps.cpp
@@ -3968,8 +3968,8 @@ class ContiguousExtractStridedSliceToExtract final
     // Avoid generating slices that have leading unit dimensions. The shape_cast
     // op that we create below would take bad generic fallback patterns
     // (ShapeCastOpRewritePattern).
-    while (sizes[numOffsets] == 1 &&
-           numOffsets < static_cast<int>(sizes.size()) - 1) {
+    while (numOffsets < static_cast<int>(sizes.size()) - 1 &&
+           sizes[numOffsets] == 1) {
       ++numOffsets;
     }
 
diff --git a/mlir/test/Dialect/Vector/canonicalize.mlir b/mlir/test/Dialect/Vector/canonicalize.mlir
index 61e858f5f226a1..0fc0140666696f 100644
--- a/mlir/test/Dialect/Vector/canonicalize.mlir
+++ b/mlir/test/Dialect/Vector/canonicalize.mlir
@@ -2932,6 +2932,17 @@ func.func @contiguous_extract_strided_slices_to_extract(%arg0 : vector<8x1x2x1x1
 
 // -----
 
+// CHECK-LABEL: @contiguous_extract_strided_slices_to_extract_no_unit_dims
+// CHECK:        %[[EXTRACT:.+]] = vector.extract {{.*}}[0, 0] : vector<4xi32> from vector<8x2x4xi32>
+// CHECK-NEXT:   return %[[EXTRACT]] :  vector<4xi32>
+func.func @contiguous_extract_strided_slices_to_extract_no_unit_dims(%arg0 : vector<8x2x4xi32>) -> vector<4xi32> {
+  %1 = vector.extract_strided_slice %arg0 {offsets = [0, 0], sizes = [1, 1], strides = [1, 1]} : vector<8x2x4xi32> to vector<1x1x4xi32>
+  %2 = vector.shape_cast %1 : vector<1x1x4xi32> to vector<4xi32>
+  return %2 : vector<4xi32>
+}
+
+// -----
+
 // CHECK-LABEL: @contiguous_extract_strided_slices_to_extract_shorter_size_list
 // CHECK:        %[[EXTRACT:.+]] = vector.extract {{.*}}[0, 0, 0, 0] : vector<1x4xi32> from vector<8x1x2x1x1x4xi32>
 // CHECK-NEXT:   return %[[EXTRACT]] :  vector<1x4xi32>

@CoTinker CoTinker requested a review from bjacob February 11, 2025 14:18
@bjacob
Copy link
Contributor

bjacob commented Feb 11, 2025

Good catch!

This PR fixes an out-of-bounds bug that occurs when there are no overlap
dimensions between the `sizes` and source of `vector.extract_strided_slice`,
causing access to `sizes` to go out of bounds.
@CoTinker CoTinker force-pushed the extract_strided_slice branch from 7067ff2 to 2489f75 Compare February 12, 2025 12:35
Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@CoTinker CoTinker merged commit 3e223e3 into llvm:main Feb 13, 2025
8 checks passed
@CoTinker CoTinker deleted the extract_strided_slice branch February 13, 2025 01:17
flovent pushed a commit to flovent/llvm-project that referenced this pull request Feb 13, 2025
This PR fixes an out-of-bounds bug that occurs when there are no overlap
dimensions between the `sizes` and source of
`vector.extract_strided_slice`, causing access to `sizes` to go out of
bounds. Fixes llvm#126196.
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
This PR fixes an out-of-bounds bug that occurs when there are no overlap
dimensions between the `sizes` and source of
`vector.extract_strided_slice`, causing access to `sizes` to go out of
bounds. Fixes llvm#126196.
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
This PR fixes an out-of-bounds bug that occurs when there are no overlap
dimensions between the `sizes` and source of
`vector.extract_strided_slice`, causing access to `sizes` to go out of
bounds. Fixes llvm#126196.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mlir] Crash when using --canonicalize

5 participants