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

[Tobias Gysi]

================
@@ -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:

It sounds a bit like both approaches are problematic. 

A stack save does not really free any memory and I am worried relaxing the side effects will cause issues down the line (e.g. is there a guarantee two free effects are not reordered)? 

Adding read/write effects on top of the allocation and free effects seems to model the effects correctly, but it is true that it may cause performance regressions since the alloca now has additional effects. I am not sure if this is too problematic though.

Any chance you could model this either with flang specific operations or handle the stack save/restore explicitly within the LICM pass? E.g. could flang have an allocation scope operation rather than using stack save restore. It sounds like even with the proposed solution you will need some quite specific logic to make sure an allocation cannot move before a free? 

TDLR I would probably lean towards the effects on the stack pointer with additional read/write effects on top of the allocation and free effects if we decide to change anything on the LLVM dialect side. 

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