[MLIR] Define memory effects for `memref.prefetch` operation #151261

AlexandreEichenberger · 2025-07-30T00:29:26Z

Currently memref.prefetch has 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

Signed-off-by: Alexandre Eichenberger <[email protected]>

llvmbot · 2025-07-30T00:29:58Z

@llvm/pr-subscribers-mlir-bufferization

@llvm/pr-subscribers-mlir

Author: Alexandre Eichenberger (AlexandreEichenberger)

Changes

Currently memref.prefetch has 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

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

2 Files Affected:

(modified) mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td (+2-1)
(modified) mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir (+9-1)

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
+}

llvmbot · 2025-07-30T00:29:58Z

@llvm/pr-subscribers-mlir-memref

Author: Alexandre Eichenberger (AlexandreEichenberger)

Changes

Currently memref.prefetch has 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

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

2 Files Affected:

(modified) mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td (+2-1)
(modified) mlir/test/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation/misc-other.mlir (+9-1)

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
+}

joker-eph · 2025-07-31T16:37:48Z

mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td

  }];

-  let arguments = (ins AnyMemRef:$memref, Variadic<Index>:$indices,
+  let arguments = (ins Arg<AnyMemRef, "prefetch address", [MemWrite]> :$memref,


Why do you need to "write to the buffer" semantically here?
(it's worth a comment, as this may not be intuitive!)

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.

It shouldn't be removing the operation if it doesn't declare any side effects at all though, and conservatively assume it has all possible side effects. It feels like there's a bug somewhere

@ftynse I believe the source of the problem is that without side-effects defined, the buffer allocation will fail:

"ops with unknown memory side effects are not supported"

So we need to add side effects, and the part about

It's basically to ensure that dead-code analysis does not remove the operation

is only an answer to "why write and not just read".

An alternative design here, more SSA-friendly, would be for the op to be side-effect free and return a token to be consumed later by some load operation.

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger · 2025-08-04T13:41:00Z

@joker-eph Page states that a review is not required to merge. Is that acceptable behavior? Want to follow the recommended procedures.

joker-eph · 2025-08-12T18:18:20Z

@joker-eph Page states that a review is not required to merge. Is that acceptable behavior? Want to follow the recommended procedures.

In general in MLIR we've been reserving "merging without review" to "trivial changes" (fixing the build, fixing a comment, other similar things like this).

mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger · 2025-08-14T14:07:23Z

I responded to comments, would it be possible to merge the PR? I don't see a merge button, presumably I don't have access to that functionality. Thanks

AlexandreEichenberger · 2025-10-03T21:02:30Z

@joker-eph can you please merge the PR. I don't seem to have permission. You approved the changes .

Thanks

Currently `memref.prefetch` has 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 --------- Signed-off-by: Alexandre Eichenberger <[email protected]>

…1261) Currently `memref.prefetch` has 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 --------- Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger added 3 commits July 29, 2025 20:12

add MemWrite side effect to memref.prefetch

7078f7d

Signed-off-by: Alexandre Eichenberger <[email protected]>

update

3f91e0f

added memref.prefetch MemWrite side effect

3a7212a

Signed-off-by: Alexandre Eichenberger <[email protected]>

llvmbot added mlir mlir:bufferization Bufferization infrastructure mlir:memref labels Jul 30, 2025

AlexandreEichenberger requested a review from ftynse July 30, 2025 15:32

Merge branch 'main' into memref-prefetch-side-effect

daaabfb

joker-eph reviewed Jul 31, 2025

View reviewed changes

AlexandreEichenberger added 2 commits August 1, 2025 10:14

update

23b37c5

added comments to responde to review

429910e

Signed-off-by: Alexandre Eichenberger <[email protected]>

AlexandreEichenberger added 4 commits August 4, 2025 09:43

Merge branch 'main' into memref-prefetch-side-effect

0c3cdcb

Merge branch 'main' into memref-prefetch-side-effect

9c410c6

Merge branch 'main' into memref-prefetch-side-effect

789c474

Merge branch 'main' into memref-prefetch-side-effect

cc864c7

AlexandreEichenberger requested a review from joker-eph August 11, 2025 20:14

joker-eph changed the title ~~Memref prefetch side effect~~ [MLIR] Define memory effects for memref.prefetch operation Aug 12, 2025

joker-eph reviewed Aug 12, 2025

View reviewed changes

mlir/include/mlir/Dialect/MemRef/IR/MemRefOps.td Outdated Show resolved Hide resolved

joker-eph approved these changes Aug 12, 2025

View reviewed changes

AlexandreEichenberger added 4 commits August 13, 2025 13:51

response to suggestions

4b0a756

Signed-off-by: Alexandre Eichenberger <[email protected]>

update

3af3996

Merge branch 'main' into memref-prefetch-side-effect

8b8d76b

Merge branch 'main' into memref-prefetch-side-effect

1fbc305

AlexandreEichenberger added 2 commits October 3, 2025 15:45

Merge branch 'main' into memref-prefetch-side-effect

07c76bf

Merge branch 'main' into memref-prefetch-side-effect

0f96d60

ftynse merged commit d45b427 into llvm:main Oct 8, 2025
9 checks passed

[MLIR] Define memory effects for memref.prefetch operation #151261

[MLIR] Define memory effects for memref.prefetch operation #151261

Uh oh!

Conversation

AlexandreEichenberger commented Jul 30, 2025

Uh oh!

llvmbot commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 30, 2025

Uh oh!

joker-eph Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

AlexandreEichenberger Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

ftynse Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

joker-eph Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

joker-eph Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

AlexandreEichenberger commented Aug 4, 2025

Uh oh!

joker-eph commented Aug 12, 2025

Uh oh!

Uh oh!

AlexandreEichenberger commented Aug 14, 2025

Uh oh!

AlexandreEichenberger commented Oct 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[MLIR] Define memory effects for `memref.prefetch` operation #151261

[MLIR] Define memory effects for `memref.prefetch` operation #151261

llvmbot commented Jul 30, 2025 •

edited

Loading