-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[mlir][memref] Remove invalid extract_aligned_pointer_as_index folding in ExpandStridedMetadata
#167615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ing in `ExpandStridedMetadata` ViewLikeOpInterface by itself doesn't guarantee to preserve the base pointer in general and `memref.view` is one such example, so limit folder to a few specific ops.
|
@llvm/pr-subscribers-mlir-memref @llvm/pr-subscribers-mlir Author: Ivan Butygin (Hardcode84) Changes
Full diff: https://github.com/llvm/llvm-project/pull/167615.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/MemRef/Transforms/ExpandStridedMetadata.cpp b/mlir/lib/Dialect/MemRef/Transforms/ExpandStridedMetadata.cpp
index bd02516d5b527..c9352e8f700d7 100644
--- a/mlir/lib/Dialect/MemRef/Transforms/ExpandStridedMetadata.cpp
+++ b/mlir/lib/Dialect/MemRef/Transforms/ExpandStridedMetadata.cpp
@@ -959,7 +959,11 @@ class RewriteExtractAlignedPointerAsIndexOfViewLikeOp
PatternRewriter &rewriter) const override {
auto viewLikeOp =
extractOp.getSource().getDefiningOp<ViewLikeOpInterface>();
- if (!viewLikeOp || extractOp.getSource() != viewLikeOp.getViewDest())
+ // ViewLikeOpInterface by itself doesn't guarantee to preserve the base
+ // pointer in general and `memref.view` is one such example, so just check
+ // for a few specific cases.
+ if (!viewLikeOp || extractOp.getSource() != viewLikeOp.getViewDest() ||
+ !isa<memref::SubViewOp, memref::ReinterpretCastOp>(viewLikeOp))
return rewriter.notifyMatchFailure(extractOp, "not a ViewLike source");
rewriter.modifyOpInPlace(extractOp, [&]() {
extractOp.getSourceMutable().assign(viewLikeOp.getViewSource());
diff --git a/mlir/test/Dialect/MemRef/expand-strided-metadata.mlir b/mlir/test/Dialect/MemRef/expand-strided-metadata.mlir
index 18cdfb73f6ba8..ec17282ed20ec 100644
--- a/mlir/test/Dialect/MemRef/expand-strided-metadata.mlir
+++ b/mlir/test/Dialect/MemRef/expand-strided-metadata.mlir
@@ -1455,3 +1455,20 @@ func.func @extract_strided_metadata_of_memory_space_cast_no_base(%base: memref<2
// CHECK-LABEL: func @extract_strided_metadata_of_memory_space_cast_no_base
// CHECK-NOT: memref.memory_space_cast
+
+// -----
+
+func.func @negative_memref_view_extract_aligned_pointer(%arg0: memref<?xi8>) -> index {
+ // `extract_aligned_pointer_as_index` must not be folded as `memref.view` can change the base pointer
+ // CHECK-LABEL: func @negative_memref_view_extract_aligned_pointer
+ // CHECK-SAME: (%[[ARG0:.*]]: memref<?xi8>)
+ // CHECK: %[[C0:.*]] = arith.constant 0 : index
+ // CHECK: %[[VIEW:.*]] = memref.view %[[ARG0]][%[[C0]]][] : memref<?xi8> to memref<f32>
+ // CHECK: %[[PTR:.*]] = memref.extract_aligned_pointer_as_index %[[VIEW]] : memref<f32> -> index
+ // CHECK: return %[[PTR]] : index
+
+ %c0 = arith.constant 0 : index
+ %0 = memref.view %arg0[%c0][] : memref<?xi8> to memref<f32>
+ %1 = memref.extract_aligned_pointer_as_index %0: memref<f32> -> index
+ return %1 : index
+}
|
krzysz00
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved modulo test updates
…ing in `ExpandStridedMetadata` (llvm#167615) `RewriteExtractAlignedPointerAsIndexOfViewLikeOp` tries to propagate `extract_aligned_pointer_as_index` through the view ops. `ViewLikeOpInterface` by itself doesn't guarantee to preserve the base pointer and `memref.view` is one such example, so limit pattern to a few specific ops.
RewriteExtractAlignedPointerAsIndexOfViewLikeOptries to propagateextract_aligned_pointer_as_indexthrough the view ops.ViewLikeOpInterfaceby itself doesn't guarantee to preserve the base pointer andmemref.viewis one such example, so limit pattern to a few specific ops.