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

[Slava Zakharin]

================
@@ -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>]>]> {
----------------
vzakhari wrote:

> Yep that is what I mean. I don't understand the requirements of Flang but it a region operation is a much nicer representation for such a scope like thing. We cannot do this in LLVM dialect since it needs to be close to LLVM IR and since we cannot really lift to such a more constraint representation during the import.

I see. Thanks for the explanation! We will try to tackle the allocation scope representation in Flang, but I am not sure how soon we can do it.

> This sounds like a good way forward to me - as mentioned above - since it seems to properly reflect what these operations do under the hood.

Thanks! Let me try to experiment with the alternative effects settings and see what pops up. I will switch this PR to draft in the meantime.

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