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

Fri Aug 15 11:38:37 PDT 2014

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/3ba56934/attachment.html>