Memref prefetch side effect #151261
Memref prefetch side effect #151261
Conversation
Signed-off-by: Alexandre Eichenberger <[email protected]>
Signed-off-by: Alexandre Eichenberger <[email protected]>
|
@llvm/pr-subscribers-mlir-bufferization @llvm/pr-subscribers-mlir Author: Alexandre Eichenberger (AlexandreEichenberger) ChangesCurrently This PR adds the needed side effect, as recommended in https://discourse.llvm.org/t/modeling-volatility-with-memory-effects/67946 This PR was created after a discussion on this specific topic here https://discourse.llvm.org/t/memref-prefetch-op-has-no-memory-side-effects-decoration-in-the-def-td-file/87482 Full diff: https://github.com/llvm/llvm-project/pull/151261.diff 2 Files Affected:
diff --git a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
index 9321089ab55fa..5a46e86eca2b5 100644
--- a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
@@ -1351,7 +1351,8 @@ def MemRef_PrefetchOp : MemRef_Op<"prefetch"> {
instruction cache.
}];
- let arguments = (ins AnyMemRef:$memref, Variadic<Index>:$indices,
+ let arguments = (ins Arg<AnyMemRef, "prefetch address", [MemWrite]> :$memref,
+ Variadic<Index>:$indices,
BoolAttr:$isWrite,
ConfinedAttr<I32Attr, [IntMinValue<0>,
IntMaxValue<3>]>:$localityHint,
diff --git a/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir b/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir
index c50c25ad8194f..fc137f1f2f722 100644
--- a/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir
+++ b/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir
@@ -17,4 +17,12 @@ func.func @func_with_assert(%arg0: index, %arg1: index) {
func.func @func_with_assume_alignment(%arg0: memref<128xi8>) {
%0 = memref.assume_alignment %arg0, 64 : memref<128xi8>
return
-}
\ No newline at end of file
+}
+
+// CHECK-LABEL: func @func_with_prefetch(
+// CHECK: memref.prefetch %arg0[%c0, %c0], read, locality<1>, data : memref<4x8xf32>
+func.func @func_with_prefetch(%arg0: memref<4x8xf32>) {
+ %c0 = arith.constant 0 : index
+ memref.prefetch %arg0[%c0, %c0], read, locality<1>, data : memref<4x8xf32>
+ return
+}
|
|
@llvm/pr-subscribers-mlir-memref Author: Alexandre Eichenberger (AlexandreEichenberger) ChangesCurrently This PR adds the needed side effect, as recommended in https://discourse.llvm.org/t/modeling-volatility-with-memory-effects/67946 This PR was created after a discussion on this specific topic here https://discourse.llvm.org/t/memref-prefetch-op-has-no-memory-side-effects-decoration-in-the-def-td-file/87482 Full diff: https://github.com/llvm/llvm-project/pull/151261.diff 2 Files Affected:
diff --git a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
index 9321089ab55fa..5a46e86eca2b5 100644
--- a/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
+++ b/mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td
@@ -1351,7 +1351,8 @@ def MemRef_PrefetchOp : MemRef_Op<"prefetch"> {
instruction cache.
}];
- let arguments = (ins AnyMemRef:$memref, Variadic<Index>:$indices,
+ let arguments = (ins Arg<AnyMemRef, "prefetch address", [MemWrite]> :$memref,
+ Variadic<Index>:$indices,
BoolAttr:$isWrite,
ConfinedAttr<I32Attr, [IntMinValue<0>,
IntMaxValue<3>]>:$localityHint,
diff --git a/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir b/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir
index c50c25ad8194f..fc137f1f2f722 100644
--- a/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir
+++ b/mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir
@@ -17,4 +17,12 @@ func.func @func_with_assert(%arg0: index, %arg1: index) {
func.func @func_with_assume_alignment(%arg0: memref<128xi8>) {
%0 = memref.assume_alignment %arg0, 64 : memref<128xi8>
return
-}
\ No newline at end of file
+}
+
+// CHECK-LABEL: func @func_with_prefetch(
+// CHECK: memref.prefetch %arg0[%c0, %c0], read, locality<1>, data : memref<4x8xf32>
+func.func @func_with_prefetch(%arg0: memref<4x8xf32>) {
+ %c0 = arith.constant 0 : index
+ memref.prefetch %arg0[%c0, %c0], read, locality<1>, data : memref<4x8xf32>
+ return
+}
|
| @@ -1351,7 +1351,8 @@ def MemRef_PrefetchOp : MemRef_Op<"prefetch"> { | |||
| instruction cache. | |||
| }]; | |||
|
|
|||
| let arguments = (ins AnyMemRef:$memref, Variadic<Index>:$indices, | |||
| let arguments = (ins Arg<AnyMemRef, "prefetch address", [MemWrite]> :$memref, | |||
There was a problem hiding this comment.
Why do you need to "write to the buffer" semantically here?
(it's worth a comment, as this may not be intuitive!)
There was a problem hiding this comment.
Good suggestion, will add a comment. It's basically to ensure that dead-code analysis does not remove the operation. It is inline with making that operation a "volatile" so that it remains in place with respect to the other load/store of the prefetched data.
Signed-off-by: Alexandre Eichenberger <[email protected]>
|
@joker-eph Page states that a review is not required to merge. Is that acceptable behavior? Want to follow the recommended procedures. |
Currently
memref.prefetchhas no memory side effects, which are necessary for some optimizations.This PR adds the needed side effect, as recommended in https://discourse.llvm.org/t/modeling-volatility-with-memory-effects/67946
This PR was created after a discussion on this specific topic here https://discourse.llvm.org/t/memref-prefetch-op-has-no-memory-side-effects-decoration-in-the-def-td-file/87482