<div dir="ltr">Yeah, sounds like you'd be better off with teaching fast isel how to lower simple tail calls.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Aug 15, 2014 at 12:53 PM, Akira Hatanaka <span dir="ltr"><<a href="mailto:ahatanaka@apple.com" target="_blank">ahatanaka@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>My understanding is that strictly speaking it’s not necessary to tail call objc functions which clang marks as “tail”, but it’s very desirable to do so, even at -O0 (see the following commits).</div>
<div><br></div><a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20121210/069653.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20121210/069653.html</a><div><a href="http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140127/098177.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140127/098177.html</a></div>
<div><br></div><div>As you’ve predicted, verifier asserts if I change those “tail” calls to “musttail” calls and run make check.</div><div><div class="h5"><div><br></div><div><div><div>On Aug 15, 2014, at 11:38 AM, Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr">Do you have more information about what kinds of things need to be tail called for correctness? Currently musttail has strict verifier requirements about function prototypes matching, which I doubt will work for <span style="font-family:arial,sans-serif;font-size:13px">objc_autoreleaseReturnValue.</span><div>
<span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">We could relax them in some cases, but I want to make sure that we can tell, based only on the LLVM IR, that it will be possible to lower the call as a tail call.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">There are two major cases to think about: caller cleanup conventions and callee cleanup conventions.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px">In caller cleanup, we can do a tail call so long as the we are passing fewer arguments than the caller pushed. At the IR level, we don't really know which args will be in registers.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px">In callee cleanup, we can always do a tail call *if* we're willing to move the return address, which makes codegen a lot harder.</span></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br>
</span></div><div><font face="arial, sans-serif">It sounds like you would have a problem if you have a void function on a i686 that needs to tail call objc_retainAutoreleaseReturnValue, which presumably takes one argument.</font></div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 14, 2014 at 4:09 PM, Bob Wilson <span dir="ltr"><<a href="mailto:bob.wilson@apple.com" target="_blank">bob.wilson@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">We should fix clang to switch over to using “musttail” attributes for those cases (<rdar://problem/18024244>). Once that is done, we can change fast-isel back to ignore “tail” attributes (<rdar://problem/18024310>).<br>
<div><div><br>
> On Aug 13, 2014, at 11:25 PM, Akira Hatanaka <<a href="mailto:ahatanaka@apple.com" target="_blank">ahatanaka@apple.com</a>> wrote:<br>
><br>
> It looks like clang marks calls as “tail” only when the called function is objc_autoreleaseReturnValue or objc_retainAutoreleaseReturnValue. Also a call is marked as “musttail” in CodeGenFunction::EmitMustTailThunk. Assuming you aren’t worried about those cases, I guess narrowing down the scope of functions that are emitted as tail calls in AArch64’s fast-isel doesn’t make much difference?<br>
><br>
> On Aug 13, 2014, at 6:03 PM, Chad Rosier <<a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a>> wrote:<br>
><br>
>> Seems musttail was landed back in April, but the Phabricator revision was<br>
>> never closed. Sounds like a good solution, but again no rush.<br>
>><br>
>>> Akira,<br>
>>> Has the musttail attribute landed? Looks like Reid was working on<br>
>>> something (<a href="http://reviews.llvm.org/D3240" target="_blank">http://reviews.llvm.org/D3240</a>), but I haven't been following<br>
>>> the patch. I have no real urgency in seeing this enhancement, so<br>
>>> prioritize this as you see fit.<br>
>>><br>
>>> Chad<br>
>>><br>
>>><br>
>>><br>
>>>> Hi Chad,<br>
>>>><br>
>>>> I’ll see if it’s possible to narrow the scope. We might be able to mark<br>
>>>> the functions that require special handling as “musttail” instead of<br>
>>>> “tail” and return false in FastISel::SelectInstruction upon seeing those<br>
>>>> functions.<br>
>>>><br>
>>>><br>
>>>>> On Aug 13, 2014, at 4:55 PM, Chad Rosier <<a href="mailto:mcrosier@codeaurora.org" target="_blank">mcrosier@codeaurora.org</a>><br>
>>>>> wrote:<br>
>>>>><br>
>>>>> Hi Akira,<br>
>>>>> Would it be possible to narrow the scope to only those tail calls that<br>
>>>>> require special handling? Or is it that the front-end will only emit<br>
>>>>> tail<br>
>>>>> calls for those calls that require it at -O0? Please clarify.<br>
>>>>><br>
>>>>> Chad<br>
>>>>><br>
>>>>>> Author: ahatanak<br>
>>>>>> Date: Wed Aug 13 18:23:58 2014<br>
>>>>>> New Revision: 215600<br>
>>>>>><br>
>>>>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215600&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=215600&view=rev</a><br>
>>>>>> Log:<br>
>>>>>> [AArch64, fast-isel] Fall back to SelectionDAG to select tail calls.<br>
>>>>>><br>
>>>>>> Certain functions such as objc_autoreleaseReturnValue have to be<br>
>>>>>> called<br>
>>>>>> as<br>
>>>>>> tail-calls even at -O0. Since normal fast-isel doesn't emit calls as<br>
>>>>>> tail<br>
>>>>>> calls,<br>
>>>>>> we have to fall back to SelectionDAG to select calls that are marked<br>
>>>>>> as<br>
>>>>>> tail.<br>
>>>>>><br>
>>>>>> <rdar://problem/17991614><br>
>>>>>><br>
>>>>>><br>
>>>>>> Added:<br>
>>>>>> llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll<br>
>>>>>> Modified:<br>
>>>>>> llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp<br>
>>>>>> llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll<br>
>>>>>><br>
>>>>>> Modified: llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp<br>
>>>>>> URL:<br>
>>>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp?rev=215600&r1=215599&r2=215600&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp?rev=215600&r1=215599&r2=215600&view=diff</a><br>
>>>>>> ==============================================================================<br>
>>>>>> --- llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp (original)<br>
>>>>>> +++ llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp Wed Aug 13<br>
>>>>>> 18:23:58<br>
>>>>>> 2014<br>
>>>>>> @@ -1687,10 +1687,15 @@ bool AArch64FastISel::FinishCall(CallLow<br>
>>>>>><br>
>>>>>> bool AArch64FastISel::FastLowerCall(CallLoweringInfo &CLI) {<br>
>>>>>> CallingConv::ID CC = CLI.CallConv;<br>
>>>>>> + bool IsTailCall = CLI.IsTailCall;<br>
>>>>>> bool IsVarArg = CLI.IsVarArg;<br>
>>>>>> const Value *Callee = CLI.Callee;<br>
>>>>>> const char *SymName = CLI.SymName;<br>
>>>>>><br>
>>>>>> + // Allow SelectionDAG isel to handle tail calls.<br>
>>>>>> + if (IsTailCall)<br>
>>>>>> + return false;<br>
>>>>>> +<br>
>>>>>> CodeModel::Model CM = TM.getCodeModel();<br>
>>>>>> // Only support the small and large code model.<br>
>>>>>> if (CM != CodeModel::Small && CM != CodeModel::Large)<br>
>>>>>><br>
>>>>>> Modified: llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll<br>
>>>>>> URL:<br>
>>>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll?rev=215600&r1=215599&r2=215600&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll?rev=215600&r1=215599&r2=215600&view=diff</a><br>
>>>>>> ==============================================================================<br>
>>>>>> --- llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll (original)<br>
>>>>>> +++ llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll Wed Aug 13<br>
>>>>>> 18:23:58<br>
>>>>>> 2014<br>
>>>>>> @@ -511,7 +511,9 @@ entry:<br>
>>>>>> ; CHECK: str {{w[0-9]+}}, [sp]<br>
>>>>>> ; FAST-LABEL: i64_split<br>
>>>>>> ; FAST: ldr x7, [{{x[0-9]+}}]<br>
>>>>>> -; FAST: str {{w[0-9]+}}, [sp]<br>
>>>>>> +; FAST: mov x[[R0:[0-9]+]], sp<br>
>>>>>> +; FAST: orr w[[R1:[0-9]+]], wzr, #0x8<br>
>>>>>> +; FAST: str w[[R1]], {{\[}}x[[R0]]{{\]}}<br>
>>>>>> %0 = load i64* bitcast (%struct.s41* @g41 to i64*), align 16<br>
>>>>>> %call = tail call i32 @callee_i64(i32 1, i32 2, i32 3, i32 4, i32 5,<br>
>>>>>> i32 6, i32 7, i64 %0, i32 8) #5<br>
>>>>>><br>
>>>>>> Added: llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll<br>
>>>>>> URL:<br>
>>>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll?rev=215600&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll?rev=215600&view=auto</a><br>
>>>>>> ==============================================================================<br>
>>>>>> --- llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll (added)<br>
>>>>>> +++ llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll Wed Aug 13<br>
>>>>>> 18:23:58 2014<br>
>>>>>> @@ -0,0 +1,11 @@<br>
>>>>>> +; RUN: llc < %s -mtriple=arm64-apple-darwin -O0 | FileCheck %s<br>
>>>>>> +<br>
>>>>>> +; CHECK: b _foo0<br>
>>>>>> +<br>
>>>>>> +define i32 @foo1() {<br>
>>>>>> +entry:<br>
>>>>>> + %call = tail call i32 @foo0()<br>
>>>>>> + ret i32 %call<br>
>>>>>> +}<br>
>>>>>> +<br>
>>>>>> +declare i32 @foo0()<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>
>>>>>><br>
>>>>><br>
>>>>><br>
>>>>> --<br>
>>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,<br>
>>>>> hosted by The Linux Foundation<br>
>>>>><br>
>>>><br>
>>>><br>
>>><br>
>>><br>
>>> --<br>
>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,<br>
>>> hosted by The Linux Foundation<br>
>>><br>
>>><br>
>><br>
>><br>
>> --<br>
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,<br>
>> hosted by The Linux Foundation<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>
<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><br></div>
</blockquote></div><br></div></div></div></div></blockquote></div><br></div>