Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit 0df9abb

Browse files
committed
Fix PR7272 in -tailcallelim instead of the inliner
The -tailcallelim pass should be checking if byval or inalloca args can be captured before marking calls as tail calls. This was the real root cause of PR7272. With a better fix in place, revert the inliner change from r105255. The test case it introduced still passes and has been moved to test/Transforms/Inline/byval-tail-call.ll. Reviewers: chandlerc Differential Revision: http://reviews.llvm.org/D3403 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@206789 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 290ce19 commit 0df9abb

File tree

4 files changed

+35
-12
lines changed

4 files changed

+35
-12
lines changed

lib/Transforms/Scalar/TailRecursionElimination.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,15 @@ bool TailCallElim::runOnFunction(Function &F) {
204204
}
205205
}
206206

207+
// If any byval or inalloca args are captured, exit. They are also allocated
208+
// in our stack frame.
209+
for (Argument &Arg : F.args()) {
210+
if (Arg.hasByValOrInAllocaAttr())
211+
PointerMayBeCaptured(&Arg, &ACT);
212+
if (ACT.Captured)
213+
return false;
214+
}
215+
207216
// Second pass, change any tail recursive calls to loops.
208217
//
209218
// FIXME: The code generator produces really bad code when an 'escaping

lib/Transforms/Utils/InlineFunction.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -586,15 +586,8 @@ bool llvm::InlineFunction(CallSite CS, InlineFunctionInfo &IFI,
586586
if (CS.isByValArgument(ArgNo)) {
587587
ActualArg = HandleByValArgument(ActualArg, TheCall, CalledFunc, IFI,
588588
CalledFunc->getParamAlignment(ArgNo+1));
589-
590-
// Calls that we inline may use the new alloca, so we need to clear
591-
// their 'tail' flags if HandleByValArgument introduced a new alloca and
592-
// the callee has calls.
593-
if (ActualArg != *AI) {
594-
MustClearTailCallFlags = true;
589+
if (ActualArg != *AI)
595590
ByValInit.push_back(std::make_pair(ActualArg, (Value*) *AI));
596-
}
597-
598591
}
599592

600593
VMap[I] = ActualArg;

test/Transforms/Inline/2010-05-31-ByvalTailcall.ll renamed to test/Transforms/Inline/byval-tail-call.ll

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
; RUN: opt < %s -tailcallelim -inline -instcombine -dse -S | FileCheck %s
22
; PR7272
33

4-
; When inlining through a byval call site, the inliner creates allocas which may
5-
; be used by inlined calls, so any inlined calls need to have their 'tail' flags
6-
; cleared. If not then you can get nastiness like with this testcase, where the
7-
; (inlined) call to 'ext' in 'foo' was being passed an uninitialized value.
4+
; Calls that capture byval parameters cannot be marked as tail calls. Other
5+
; tails that don't capture byval parameters can still be tail calls.
86

97
target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32"
108
target triple = "i386-pc-linux-gnu"
@@ -23,3 +21,18 @@ define void @foo(i32* %x) {
2321
call void @bar(i32* byval %x)
2422
ret void
2523
}
24+
25+
define internal void @qux(i32* byval %x) {
26+
call void @ext(i32* %x)
27+
tail call void @ext(i32* null)
28+
ret void
29+
}
30+
define void @frob(i32* %x) {
31+
; CHECK-LABEL: define void @frob(
32+
; CHECK: alloca i32
33+
; CHECK: {{^ *}}call void @ext(
34+
; CHECK: tail call void @ext(i32* null)
35+
; CHECK: ret void
36+
tail call void @qux(i32* byval %x)
37+
ret void
38+
}

test/Transforms/TailCallElim/basic.ll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,11 @@ cond_false:
143143
call void @noarg()
144144
ret i32* null
145145
}
146+
147+
; Don't tail call if a byval arg is captured.
148+
define void @test9(i32* byval %a) {
149+
; CHECK-LABEL: define void @test9(
150+
; CHECK: {{^ *}}call void @use(
151+
call void @use(i32* %a)
152+
ret void
153+
}

0 commit comments

Comments
 (0)