<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Oct 17, 2014 at 12:56 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On 17 October 2014 12:17, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">In fact this was PR7272.<br></blockquote><div><br></div></span><div>PR7272 is the other way around?</div><div><br></div><div><div>define void @foo(i32* %x) {</div><div> tail call void @bar(i32* byval %x) ;; bad tail from PR7272</div><div> ret void</div><div>}</div></div><div><br></div><div><div><div>define void @foo(i32* byval %x) {</div><div> tail call void @bar(i32* %x) ;; okay tail from this patch</div><div> ret void</div><div>}</div></div></div></div></div></div></blockquote><div><br></div><div>This is precisely the case when it's not safe to tail call. You're capturing the address of something allocated on the current stack frame and passing it down, which you can't do.</div><div><br></div><div>The PR7272 case doesn't have that problem. It could hypothetically be lowered correctly, and IMO should be rejected by the backend instead of the middle end.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>At least I think so?</div><div><div class="h5"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div>On 17 October 2014 15:14, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br>
> Is this actually safe?<br>
><br>
> The caller will deallocate the stack and then pass a pointer to that<br>
> area to the callee...<br>
><br>
> On 15 October 2014 23:27, Saleem Abdulrasool <<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>> wrote:<br>
>> Author: compnerd<br>
>> Date: Wed Oct 15 22:27:30 2014<br>
>> New Revision: 219899<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219899&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=219899&view=rev</a><br>
>> Log:<br>
>> TRE: make TRE a bit more aggressive<br>
>><br>
>> Make tail recursion elimination a bit more aggressive. This allows us to get<br>
>> tail recursion on functions that are just branches to a different function. The<br>
>> fact that the function takes a byval argument does not restrict it from being<br>
>> optimised into just a tail call.<br>
>><br>
>> Added:<br>
>> llvm/trunk/test/Transforms/TailCallElim/byval.ll<br>
>> Modified:<br>
>> llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp<br>
>> llvm/trunk/test/Transforms/Inline/byval-tail-call.ll<br>
>> llvm/trunk/test/Transforms/TailCallElim/basic.ll<br>
>><br>
>> Modified: llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=219899&r1=219898&r2=219899&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp?rev=219899&r1=219898&r2=219899&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp (original)<br>
>> +++ llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp Wed Oct 15 22:27:30 2014<br>
>> @@ -249,12 +249,7 @@ bool TailCallElim::markTails(Function &F<br>
>> return false;<br>
>> AllCallsAreTailCalls = true;<br>
>><br>
>> - // The local stack holds all alloca instructions and all byval arguments.<br>
>> AllocaDerivedValueTracker Tracker;<br>
>> - for (Argument &Arg : F.args()) {<br>
>> - if (Arg.hasByValAttr())<br>
>> - Tracker.walk(&Arg);<br>
>> - }<br>
>> for (auto &BB : F) {<br>
>> for (auto &I : BB)<br>
>> if (AllocaInst *AI = dyn_cast<AllocaInst>(&I))<br>
>> @@ -310,9 +305,8 @@ bool TailCallElim::markTails(Function &F<br>
>> for (auto &Arg : CI->arg_operands()) {<br>
>> if (isa<Constant>(Arg.getUser()))<br>
>> continue;<br>
>> - if (Argument *A = dyn_cast<Argument>(Arg.getUser()))<br>
>> - if (!A->hasByValAttr())<br>
>> - continue;<br>
>> + if (isa<Argument>(Arg.getUser()))<br>
>> + continue;<br>
>> SafeToTail = false;<br>
>> break;<br>
>> }<br>
>><br>
>> Modified: llvm/trunk/test/Transforms/Inline/byval-tail-call.ll<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/byval-tail-call.ll?rev=219899&r1=219898&r2=219899&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/byval-tail-call.ll?rev=219899&r1=219898&r2=219899&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Transforms/Inline/byval-tail-call.ll (original)<br>
>> +++ llvm/trunk/test/Transforms/Inline/byval-tail-call.ll Wed Oct 15 22:27:30 2014<br>
>> @@ -27,10 +27,11 @@ define internal void @qux(i32* byval %x)<br>
>> tail call void @ext(i32* null)<br>
>> ret void<br>
>> }<br>
>> +<br>
>> define void @frob(i32* %x) {<br>
>> ; CHECK-LABEL: define void @frob(<br>
>> ; CHECK: alloca i32<br>
>> -; CHECK: {{^ *}}call void @ext(<br>
>> +; CHECK: {{^ *}}tail call void @ext(<br>
>> ; CHECK: tail call void @ext(i32* null)<br>
>> ; CHECK: ret void<br>
>> tail call void @qux(i32* byval %x)<br>
>><br>
>> Modified: llvm/trunk/test/Transforms/TailCallElim/basic.ll<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/basic.ll?rev=219899&r1=219898&r2=219899&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/basic.ll?rev=219899&r1=219898&r2=219899&view=diff</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Transforms/TailCallElim/basic.ll (original)<br>
>> +++ llvm/trunk/test/Transforms/TailCallElim/basic.ll Wed Oct 15 22:27:30 2014<br>
>> @@ -147,7 +147,7 @@ cond_false:<br>
>> ; Don't tail call if a byval arg is captured.<br>
>> define void @test9(i32* byval %a) {<br>
>> ; CHECK-LABEL: define void @test9(<br>
>> -; CHECK: {{^ *}}call void @use(<br>
>> +; CHECK: {{^ *}}tail call void @use(<br>
>> call void @use(i32* %a)<br>
>> ret void<br>
>> }<br>
>><br>
>> Added: llvm/trunk/test/Transforms/TailCallElim/byval.ll<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/byval.ll?rev=219899&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/TailCallElim/byval.ll?rev=219899&view=auto</a><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Transforms/TailCallElim/byval.ll (added)<br>
>> +++ llvm/trunk/test/Transforms/TailCallElim/byval.ll Wed Oct 15 22:27:30 2014<br>
>> @@ -0,0 +1,34 @@<br>
>> +; RUN: opt -mtriple i386 -Os -S %s -o - | FileCheck %s<br>
>> +; RUN: opt -mtriple x86_64 -Os -S %s -o - | FileCheck %s<br>
>> +; RUN: opt -mtriple armv7 -Os -S %s -o - | FileCheck %s<br>
>> +<br>
>> +%struct.D16 = type { [16 x double] }<br>
>> +<br>
>> +declare void @_Z2OpP3D16PKS_S2_(%struct.D16*, %struct.D16*, %struct.D16*)<br>
>> +<br>
>> +define void @_Z7TestRefRK3D16S1_(%struct.D16* noalias sret %agg.result, %struct.D16* %RHS, %struct.D16* %LHS) {<br>
>> + %1 = alloca %struct.D16*, align 8<br>
>> + %2 = alloca %struct.D16*, align 8<br>
>> + store %struct.D16* %RHS, %struct.D16** %1, align 8<br>
>> + store %struct.D16* %LHS, %struct.D16** %2, align 8<br>
>> + %3 = load %struct.D16** %1, align 8<br>
>> + %4 = load %struct.D16** %2, align 8<br>
>> + call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %3, %struct.D16* %4)<br>
>> + ret void<br>
>> +}<br>
>> +<br>
>> +; CHECK: define void @_Z7TestRefRK3D16S1_({{.*}}) {<br>
>> +; CHECK: tail call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %RHS, %struct.D16* %LHS)<br>
>> +; CHECK: ret void<br>
>> +; CHECK: }<br>
>> +<br>
>> +define void @_Z7TestVal3D16S_(%struct.D16* noalias sret %agg.result, %struct.D16* byval align 8 %RHS, %struct.D16* byval align 8 %LHS) {<br>
>> + call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %RHS, %struct.D16* %LHS)<br>
>> + ret void<br>
>> +}<br>
>> +<br>
>> +; CHECK: define void @_Z7TestVal3D16S_({{.*}}) {<br>
>> +; CHECK: tail call void @_Z2OpP3D16PKS_S2_(%struct.D16* %agg.result, %struct.D16* %RHS, %struct.D16* %LHS)<br>
>> +; CHECK: ret void<br>
>> +; CHECK: }<br>
>> +<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>