[llvm] [TailCallElim] Don’t mark llvm.stackrestore with tail-call (PR #101352)
Shimin Cui via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 4 18:53:17 PDT 2024
https://github.com/scui-ibm updated https://github.com/llvm/llvm-project/pull/101352
>From 89cdac4d3acb52b3f0332e05de45093eb9fadd80 Mon Sep 17 00:00:00 2001
From: Shimin Cui <scui at ca.ibm.com>
Date: Wed, 31 Jul 2024 11:24:43 -0400
Subject: [PATCH 1/4] [BasicAA] MemCpyOpt fix on tail stackrestore
---
llvm/lib/Analysis/BasicAliasAnalysis.cpp | 12 ++++++------
llvm/test/Transforms/MemCpyOpt/stackrestore.ll | 9 +++++----
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index e474899fb548e..504026e44f531 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -914,6 +914,12 @@ ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call,
const Value *Object = getUnderlyingObject(Loc.Ptr);
+ // Stack restore is able to modify unescaped dynamic allocas. Assume it may
+ // modify them even though the alloca is not escaped.
+ if (auto *AI = dyn_cast<AllocaInst>(Object))
+ if (!AI->isStaticAlloca() && isIntrinsicCall(Call, Intrinsic::stackrestore))
+ return ModRefInfo::Mod;
+
// Calls marked 'tail' cannot read or write allocas from the current frame
// because the current frame might be destroyed by the time they run. However,
// a tail call may use an alloca with byval. Calling with byval copies the
@@ -925,12 +931,6 @@ ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call,
!CI->getAttributes().hasAttrSomewhere(Attribute::ByVal))
return ModRefInfo::NoModRef;
- // Stack restore is able to modify unescaped dynamic allocas. Assume it may
- // modify them even though the alloca is not escaped.
- if (auto *AI = dyn_cast<AllocaInst>(Object))
- if (!AI->isStaticAlloca() && isIntrinsicCall(Call, Intrinsic::stackrestore))
- return ModRefInfo::Mod;
-
// A call can access a locally allocated object either because it is passed as
// an argument to the call, or because it has escaped prior to the call.
//
diff --git a/llvm/test/Transforms/MemCpyOpt/stackrestore.ll b/llvm/test/Transforms/MemCpyOpt/stackrestore.ll
index 0fc37c44fa9e8..f5feea98e21cd 100644
--- a/llvm/test/Transforms/MemCpyOpt/stackrestore.ll
+++ b/llvm/test/Transforms/MemCpyOpt/stackrestore.ll
@@ -1,5 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=memcpyopt < %s -verify-memoryssa | FileCheck %s
+; RUN: opt -S -passes="tailcallelim,memcpyopt" < %s -verify-memoryssa | FileCheck %s
; PR40118: BasicAA didn't realize that stackrestore ends the lifetime of
; unescaped dynamic allocas, such as those that might come from inalloca.
@@ -23,7 +24,7 @@ define i32 @test_norestore(i32 %n) {
; CHECK-NEXT: store i8 0, ptr [[P10]], align 1
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[TMPMEM]], ptr [[P]], i32 10, i1 false)
; CHECK-NEXT: call void @external()
-; CHECK-NEXT: [[HEAP:%.*]] = call ptr @malloc(i32 9)
+; CHECK-NEXT: [[HEAP:%.*]] = tail call ptr @malloc(i32 9)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[HEAP]], ptr align 1 @str, i32 9, i1 false)
; CHECK-NEXT: call void @useit(ptr [[HEAP]])
; CHECK-NEXT: ret i32 0
@@ -40,7 +41,7 @@ define i32 @test_norestore(i32 %n) {
call void @llvm.memcpy.p0.p0.i32(ptr %tmpmem, ptr %p, i32 10, i1 false)
call void @external()
- %heap = call ptr @malloc(i32 9)
+ %heap = tail call ptr @malloc(i32 9)
call void @llvm.memcpy.p0.p0.i32(ptr %heap, ptr %tmpmem, i32 9, i1 false)
call void @useit(ptr %heap)
ret i32 0
@@ -58,7 +59,7 @@ define i32 @test_stackrestore() {
; CHECK-NEXT: store i8 0, ptr [[P10]], align 1
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[TMPMEM]], ptr [[ARGMEM]], i32 10, i1 false)
; CHECK-NEXT: call void @llvm.stackrestore.p0(ptr [[INALLOCA_SAVE]])
-; CHECK-NEXT: [[HEAP:%.*]] = call ptr @malloc(i32 9)
+; CHECK-NEXT: [[HEAP:%.*]] = tail call ptr @malloc(i32 9)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[HEAP]], ptr [[TMPMEM]], i32 9, i1 false)
; CHECK-NEXT: call void @useit(ptr [[HEAP]])
; CHECK-NEXT: ret i32 0
@@ -74,7 +75,7 @@ define i32 @test_stackrestore() {
call void @llvm.memcpy.p0.p0.i32(ptr %tmpmem, ptr %argmem, i32 10, i1 false)
call void @llvm.stackrestore(ptr %inalloca.save)
- %heap = call ptr @malloc(i32 9)
+ %heap = tail call ptr @malloc(i32 9)
call void @llvm.memcpy.p0.p0.i32(ptr %heap, ptr %tmpmem, i32 9, i1 false)
call void @useit(ptr %heap)
ret i32 0
>From 23bce05a8c7ce8e3c88f46a11a2d1dccf4745d68 Mon Sep 17 00:00:00 2001
From: Shimin Cui <scui at ca.ibm.com>
Date: Fri, 2 Aug 2024 12:41:14 -0400
Subject: [PATCH 2/4] Revert "[BasicAA] MemCpyOpt fix on tail stackrestore"
This reverts commit 89cdac4d3acb52b3f0332e05de45093eb9fadd80.
---
llvm/lib/Analysis/BasicAliasAnalysis.cpp | 12 ++++++------
llvm/test/Transforms/MemCpyOpt/stackrestore.ll | 9 ++++-----
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 504026e44f531..e474899fb548e 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -914,12 +914,6 @@ ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call,
const Value *Object = getUnderlyingObject(Loc.Ptr);
- // Stack restore is able to modify unescaped dynamic allocas. Assume it may
- // modify them even though the alloca is not escaped.
- if (auto *AI = dyn_cast<AllocaInst>(Object))
- if (!AI->isStaticAlloca() && isIntrinsicCall(Call, Intrinsic::stackrestore))
- return ModRefInfo::Mod;
-
// Calls marked 'tail' cannot read or write allocas from the current frame
// because the current frame might be destroyed by the time they run. However,
// a tail call may use an alloca with byval. Calling with byval copies the
@@ -931,6 +925,12 @@ ModRefInfo BasicAAResult::getModRefInfo(const CallBase *Call,
!CI->getAttributes().hasAttrSomewhere(Attribute::ByVal))
return ModRefInfo::NoModRef;
+ // Stack restore is able to modify unescaped dynamic allocas. Assume it may
+ // modify them even though the alloca is not escaped.
+ if (auto *AI = dyn_cast<AllocaInst>(Object))
+ if (!AI->isStaticAlloca() && isIntrinsicCall(Call, Intrinsic::stackrestore))
+ return ModRefInfo::Mod;
+
// A call can access a locally allocated object either because it is passed as
// an argument to the call, or because it has escaped prior to the call.
//
diff --git a/llvm/test/Transforms/MemCpyOpt/stackrestore.ll b/llvm/test/Transforms/MemCpyOpt/stackrestore.ll
index f5feea98e21cd..0fc37c44fa9e8 100644
--- a/llvm/test/Transforms/MemCpyOpt/stackrestore.ll
+++ b/llvm/test/Transforms/MemCpyOpt/stackrestore.ll
@@ -1,6 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt -S -passes=memcpyopt < %s -verify-memoryssa | FileCheck %s
-; RUN: opt -S -passes="tailcallelim,memcpyopt" < %s -verify-memoryssa | FileCheck %s
; PR40118: BasicAA didn't realize that stackrestore ends the lifetime of
; unescaped dynamic allocas, such as those that might come from inalloca.
@@ -24,7 +23,7 @@ define i32 @test_norestore(i32 %n) {
; CHECK-NEXT: store i8 0, ptr [[P10]], align 1
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[TMPMEM]], ptr [[P]], i32 10, i1 false)
; CHECK-NEXT: call void @external()
-; CHECK-NEXT: [[HEAP:%.*]] = tail call ptr @malloc(i32 9)
+; CHECK-NEXT: [[HEAP:%.*]] = call ptr @malloc(i32 9)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[HEAP]], ptr align 1 @str, i32 9, i1 false)
; CHECK-NEXT: call void @useit(ptr [[HEAP]])
; CHECK-NEXT: ret i32 0
@@ -41,7 +40,7 @@ define i32 @test_norestore(i32 %n) {
call void @llvm.memcpy.p0.p0.i32(ptr %tmpmem, ptr %p, i32 10, i1 false)
call void @external()
- %heap = tail call ptr @malloc(i32 9)
+ %heap = call ptr @malloc(i32 9)
call void @llvm.memcpy.p0.p0.i32(ptr %heap, ptr %tmpmem, i32 9, i1 false)
call void @useit(ptr %heap)
ret i32 0
@@ -59,7 +58,7 @@ define i32 @test_stackrestore() {
; CHECK-NEXT: store i8 0, ptr [[P10]], align 1
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[TMPMEM]], ptr [[ARGMEM]], i32 10, i1 false)
; CHECK-NEXT: call void @llvm.stackrestore.p0(ptr [[INALLOCA_SAVE]])
-; CHECK-NEXT: [[HEAP:%.*]] = tail call ptr @malloc(i32 9)
+; CHECK-NEXT: [[HEAP:%.*]] = call ptr @malloc(i32 9)
; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr [[HEAP]], ptr [[TMPMEM]], i32 9, i1 false)
; CHECK-NEXT: call void @useit(ptr [[HEAP]])
; CHECK-NEXT: ret i32 0
@@ -75,7 +74,7 @@ define i32 @test_stackrestore() {
call void @llvm.memcpy.p0.p0.i32(ptr %tmpmem, ptr %argmem, i32 10, i1 false)
call void @llvm.stackrestore(ptr %inalloca.save)
- %heap = tail call ptr @malloc(i32 9)
+ %heap = call ptr @malloc(i32 9)
call void @llvm.memcpy.p0.p0.i32(ptr %heap, ptr %tmpmem, i32 9, i1 false)
call void @useit(ptr %heap)
ret i32 0
>From b8f03748aae662cdcb53bcf94a9909e2da44bbd4 Mon Sep 17 00:00:00 2001
From: Shimin Cui <scui at ca.ibm.com>
Date: Fri, 2 Aug 2024 12:42:12 -0400
Subject: [PATCH 3/4] Don't mark tail call for stackrestore
---
.../Scalar/TailRecursionElimination.cpp | 5 +++++
.../Transforms/TailCallElim/stackrestore.ll | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 llvm/test/Transforms/TailCallElim/stackrestore.ll
diff --git a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
index 1b3e6d9549b82..216cb62541c3a 100644
--- a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -243,6 +243,11 @@ static bool markTails(Function &F, OptimizationRemarkEmitter *ORE) {
isa<PseudoProbeInst>(&I))
continue;
+ // Bail out intrinsic stackrestore call.
+ if (auto *II = dyn_cast<IntrinsicInst>(CI))
+ if (II->getIntrinsicID() == Intrinsic::stackrestore)
+ continue;
+
// Special-case operand bundles "clang.arc.attachedcall", "ptrauth", and
// "kcfi".
bool IsNoTail = CI->isNoTailCall() ||
diff --git a/llvm/test/Transforms/TailCallElim/stackrestore.ll b/llvm/test/Transforms/TailCallElim/stackrestore.ll
new file mode 100644
index 0000000000000..2a8b4e3955673
--- /dev/null
+++ b/llvm/test/Transforms/TailCallElim/stackrestore.ll
@@ -0,0 +1,17 @@
+; RUN: opt -S -passes=tailcallelim < %s | FileCheck %s
+
+target datalayout = "E-m:a-Fi32-i64:64-p:32:32-n32"
+target triple = "powerpc-ibm-aix7.2.0.0"
+
+define void @foo() {
+; CHECK-LABEL: define void @foo()
+; CHECK-NOT: tail call void @llvm.stackrestore.p0
+;
+entry:
+ %0 = call ptr @llvm.stacksave.p0()
+ call void @llvm.stackrestore.p0(ptr %0)
+ ret void
+}
+
+declare ptr @llvm.stacksave.p0()
+declare void @llvm.stackrestore.p0(ptr)
>From 8aa44bed70bebb65c99b5697a82b38fac62e9485 Mon Sep 17 00:00:00 2001
From: Shimin Cui <scui at ca.ibm.com>
Date: Sun, 4 Aug 2024 21:52:01 -0400
Subject: [PATCH 4/4] Fix comments
---
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
index 216cb62541c3a..bdcccbe3550ec 100644
--- a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -243,7 +243,8 @@ static bool markTails(Function &F, OptimizationRemarkEmitter *ORE) {
isa<PseudoProbeInst>(&I))
continue;
- // Bail out intrinsic stackrestore call.
+ // Bail out intrinsic stackrestore call because it can modify unescaped
+ // allocas.
if (auto *II = dyn_cast<IntrinsicInst>(CI))
if (II->getIntrinsicID() == Intrinsic::stackrestore)
continue;
More information about the llvm-commits
mailing list