[llvm] r215600 - [AArch64, fast-isel] Fall back to SelectionDAG to select tail calls.

Fri Aug 15 13:50:21 PDT 2014

Yeah, sounds like you'd be better off with teaching fast isel how to lower
simple tail calls.

On Fri, Aug 15, 2014 at 12:53 PM, Akira Hatanaka <ahatanaka at apple.com>
wrote:

> 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).
>
>
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20121210/069653.html
>
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140127/098177.html
>
> As you’ve predicted, verifier asserts if I change those “tail” calls to
> “musttail” calls and run make check.
>
> On Aug 15, 2014, at 11:38 AM, Reid Kleckner <rnk at google.com> wrote:
>
> 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
> objc_autoreleaseReturnValue.
>
> 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.
>
> There are two major cases to think about: caller cleanup conventions and
> callee cleanup conventions.
> 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.
> 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.
>
> 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.
>
>
> On Thu, Aug 14, 2014 at 4:09 PM, Bob Wilson <bob.wilson at apple.com> wrote:
>
>> 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>).
>>
>> > On Aug 13, 2014, at 11:25 PM, Akira Hatanaka <ahatanaka at apple.com>
>> wrote:
>> >
>> > 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?
>> >
>> > On Aug 13, 2014, at 6:03 PM, Chad Rosier <mcrosier at codeaurora.org>
>> wrote:
>> >
>> >> Seems musttail was landed back in April, but the Phabricator revision
>> was
>> >> never closed.  Sounds like a good solution, but again no rush.
>> >>
>> >>> Akira,
>> >>> Has the musttail attribute landed?  Looks like Reid was working on
>> >>> something (http://reviews.llvm.org/D3240), but I haven't been
>> following
>> >>> the patch.  I have no real urgency in seeing this enhancement, so
>> >>> prioritize this as you see fit.
>> >>>
>> >>> Chad
>> >>>
>> >>>
>> >>>
>> >>>> Hi Chad,
>> >>>>
>> >>>> I’ll see if it’s possible to narrow the scope. We might be able to
>> mark
>> >>>> the functions that require special handling as “musttail” instead of
>> >>>> “tail” and return false in FastISel::SelectInstruction upon seeing
>> those
>> >>>> functions.
>> >>>>
>> >>>>
>> >>>>> On Aug 13, 2014, at 4:55 PM, Chad Rosier <mcrosier at codeaurora.org>
>> >>>>> wrote:
>> >>>>>
>> >>>>> Hi Akira,
>> >>>>> Would it be possible to narrow the scope to only those tail calls
>> that
>> >>>>> require special handling?  Or is it that the front-end will only
>> emit
>> >>>>> tail
>> >>>>> calls for those calls that require it at -O0?  Please clarify.
>> >>>>>
>> >>>>> Chad
>> >>>>>
>> >>>>>> Author: ahatanak
>> >>>>>> Date: Wed Aug 13 18:23:58 2014
>> >>>>>> New Revision: 215600
>> >>>>>>
>> >>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=215600&view=rev
>> >>>>>> Log:
>> >>>>>> [AArch64, fast-isel] Fall back to SelectionDAG to select tail
>> calls.
>> >>>>>>
>> >>>>>> Certain functions such as objc_autoreleaseReturnValue have to be
>> >>>>>> called
>> >>>>>> as
>> >>>>>> tail-calls even at -O0. Since normal fast-isel doesn't emit calls
>> as
>> >>>>>> tail
>> >>>>>> calls,
>> >>>>>> we have to fall back to SelectionDAG to select calls that are
>> marked
>> >>>>>> as
>> >>>>>> tail.
>> >>>>>>
>> >>>>>> <rdar://problem/17991614>
>> >>>>>>
>> >>>>>>
>> >>>>>> Added:
>> >>>>>>  llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll
>> >>>>>> Modified:
>> >>>>>>  llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp
>> >>>>>>  llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll
>> >>>>>>
>> >>>>>> Modified: llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp
>> >>>>>> URL:
>> >>>>>>
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp?rev=215600&r1=215599&r2=215600&view=diff
>> >>>>>>
>> ==============================================================================
>> >>>>>> --- llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp (original)
>> >>>>>> +++ llvm/trunk/lib/Target/AArch64/AArch64FastISel.cpp Wed Aug 13
>> >>>>>> 18:23:58
>> >>>>>> 2014
>> >>>>>> @@ -1687,10 +1687,15 @@ bool AArch64FastISel::FinishCall(CallLow
>> >>>>>>
>> >>>>>> bool AArch64FastISel::FastLowerCall(CallLoweringInfo &CLI) {
>> >>>>>> CallingConv::ID CC  = CLI.CallConv;
>> >>>>>> +  bool IsTailCall     = CLI.IsTailCall;
>> >>>>>> bool IsVarArg       = CLI.IsVarArg;
>> >>>>>> const Value *Callee = CLI.Callee;
>> >>>>>> const char *SymName = CLI.SymName;
>> >>>>>>
>> >>>>>> +  // Allow SelectionDAG isel to handle tail calls.
>> >>>>>> +  if (IsTailCall)
>> >>>>>> +    return false;
>> >>>>>> +
>> >>>>>> CodeModel::Model CM = TM.getCodeModel();
>> >>>>>> // Only support the small and large code model.
>> >>>>>> if (CM != CodeModel::Small && CM != CodeModel::Large)
>> >>>>>>
>> >>>>>> Modified: llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll
>> >>>>>> URL:
>> >>>>>>
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll?rev=215600&r1=215599&r2=215600&view=diff
>> >>>>>>
>> ==============================================================================
>> >>>>>> --- llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll (original)
>> >>>>>> +++ llvm/trunk/test/CodeGen/AArch64/arm64-abi_align.ll Wed Aug 13
>> >>>>>> 18:23:58
>> >>>>>> 2014
>> >>>>>> @@ -511,7 +511,9 @@ entry:
>> >>>>>> ; CHECK: str {{w[0-9]+}}, [sp]
>> >>>>>> ; FAST-LABEL: i64_split
>> >>>>>> ; FAST: ldr x7, [{{x[0-9]+}}]
>> >>>>>> -; FAST: str {{w[0-9]+}}, [sp]
>> >>>>>> +; FAST: mov x[[R0:[0-9]+]], sp
>> >>>>>> +; FAST: orr w[[R1:[0-9]+]], wzr, #0x8
>> >>>>>> +; FAST: str w[[R1]], {{\[}}x[[R0]]{{\]}}
>> >>>>>> %0 = load i64* bitcast (%struct.s41* @g41 to i64*), align 16
>> >>>>>> %call = tail call i32 @callee_i64(i32 1, i32 2, i32 3, i32 4, i32
>> 5,
>> >>>>>>                                   i32 6, i32 7, i64 %0, i32 8) #5
>> >>>>>>
>> >>>>>> Added: llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll
>> >>>>>> URL:
>> >>>>>>
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll?rev=215600&view=auto
>> >>>>>>
>> ==============================================================================
>> >>>>>> --- llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll (added)
>> >>>>>> +++ llvm/trunk/test/CodeGen/AArch64/tailcall-fastisel.ll Wed Aug 13
>> >>>>>> 18:23:58 2014
>> >>>>>> @@ -0,0 +1,11 @@
>> >>>>>> +; RUN: llc < %s -mtriple=arm64-apple-darwin -O0 | FileCheck %s
>> >>>>>> +
>> >>>>>> +; CHECK: b _foo0
>> >>>>>> +
>> >>>>>> +define i32 @foo1() {
>> >>>>>> +entry:
>> >>>>>> +  %call = tail call i32 @foo0()
>> >>>>>> +  ret i32 %call
>> >>>>>> +}
>> >>>>>> +
>> >>>>>> +declare i32 @foo0()
>> >>>>>>
>> >>>>>>
>> >>>>>> _______________________________________________
>> >>>>>> llvm-commits mailing list
>> >>>>>> llvm-commits at cs.uiuc.edu
>> >>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >>>>>>
>> >>>>>
>> >>>>>
>> >>>>> --
>> >>>>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> >>>>> hosted by The Linux Foundation
>> >>>>>
>> >>>>
>> >>>>
>> >>>
>> >>>
>> >>> --
>> >>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> >>> hosted by The Linux Foundation
>> >>>
>> >>>
>> >>
>> >>
>> >> --
>> >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> >> hosted by The Linux Foundation
>> >>
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140815/e92b5d8a/attachment.html>