[llvm] [TailCallElim] Don’t mark llvm.stackrestore with tail-call (PR #101352)
Shimin Cui via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 5 08:44:44 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/5] [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/5] 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/5] 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/5] 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;
>From 3fd2b2b3380a7e62af786b3b09b9de34122d2ecf Mon Sep 17 00:00:00 2001
From: Shimin Cui <scui at ca.ibm.com>
Date: Mon, 5 Aug 2024 11:43:05 -0400
Subject: [PATCH 5/5] Minor change to address review comments
---
llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp | 4 ++--
llvm/test/Transforms/TailCallElim/stackrestore.ll | 3 ---
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
index bdcccbe3550ec..53e486f3dc6cd 100644
--- a/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -243,8 +243,8 @@ static bool markTails(Function &F, OptimizationRemarkEmitter *ORE) {
isa<PseudoProbeInst>(&I))
continue;
- // Bail out intrinsic stackrestore call because it can modify unescaped
- // allocas.
+ // Bail out for intrinsic stackrestore call because it can modify
+ // unescaped allocas.
if (auto *II = dyn_cast<IntrinsicInst>(CI))
if (II->getIntrinsicID() == Intrinsic::stackrestore)
continue;
diff --git a/llvm/test/Transforms/TailCallElim/stackrestore.ll b/llvm/test/Transforms/TailCallElim/stackrestore.ll
index 2a8b4e3955673..55b1ec933e1ec 100644
--- a/llvm/test/Transforms/TailCallElim/stackrestore.ll
+++ b/llvm/test/Transforms/TailCallElim/stackrestore.ll
@@ -1,8 +1,5 @@
; 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
More information about the llvm-commits
mailing list