[flang-commits] [flang] [mlir] [mlir][LLVMIR] Set memory effects for llvm.intrin.stacksave/restore. (PR #191918)

[Tobias Gysi via flang-commits]

================
@@ -893,11 +893,36 @@ def LLVM_EhTypeidForOp : LLVM_OneResultIntrOp<"eh.typeid.for", [], [0]> {
 // Stack save/restore intrinsics.
 //
 
-def LLVM_StackSaveOp : LLVM_OneResultIntrOp<"stacksave", [0]> {
+// Technically, llvm.intrin.stacksave does not free the stack,
+// but MemFree<AutomaticAllocationScopeResource> seems to be
+// an appropriate effect to guarantee that the allocations
+// between stacksave/stackrestore cannot be moved out of
+// the stacksave/stackrestore "scope".
+// For example,
+//   %ptr = llvm.intrin.stacksave : !llvm.ptr
+//   %alloca = llvm.alloca %zero x i32 : (i32) -> !llvm.ptr
+//   ... = llvm.load %alloca : !llvm.ptr -> i32
+//   llvm.store %x, %alloca : i32, !llvm.ptr
+//   llvm.intrin.stackrestore %ptr : !llvm.ptr
+//
+// The 'llvm.alloca' should not be movable neither before
+// llvm.intrin.stacksave nor after llvm.intrin.stackrestore.
+// Moreover, llvm.intrin.stackrestore cannot be moved
+// above the load the the store.
+//
+// The 'Free' effect on 'AutomaticAllocationScopeResource'
+// for both stacksave and stackrestore should guarantee that.
+def LLVM_StackSaveOp
+    : LLVM_OneResultIntrOp<
+          "stacksave", [0], [],
+          [MemoryEffects<[MemFree<AutomaticAllocationScopeResource>]>]> {
----------------
gysit wrote:

> Good question, and I would think they should be reorderable. At least, they should be reorderable if they apply to different non-aliasing "values". When there is no "value" in the effect it is unclear if we should allow reordering. @joker-eph do you have any thoughts on that point?

I agree that in the general case allocation and free effects should be reorderable, which is another argument to model the accesses to the stack pointer as memory effects I think. 

https://github.com/llvm/llvm-project/pull/191918