[llvm-commits] [PATCH][code size][ARM] Emit regular call instructions instead of the move, branch sequence

[Jim Grosbach]

Hi Quentin,

This looks good to me, with one very minor stylistic tweak below.

Regards,
  Jim

> Index: lib/Target/ARM/ARMISelLowering.cpp
> ===================================================================
> --- lib/Target/ARM/ARMISelLowering.cpp	(revision 166831)
> +++ lib/Target/ARM/ARMISelLowering.cpp	(working copy)
> @@ -1594,11 +1594,14 @@
>  
>    // FIXME: handle tail calls differently.
>    unsigned CallOpc;
> +  Attributes FnAttributes = MF.getFunction()->getFnAttributes();

Since we're only using this to check for the force-size attribute, the code will probably read a bit more clearly if we just make a bool here for that. Something like:

bool HasForceSizeAttr = FnAttributes = MF.getFunction()->getFnAttributes().hasAttribute(Attributes::ForceSizeOpt);

>    if (Subtarget->isThumb()) {
>      if ((!isDirect || isARMFunc) && !Subtarget->hasV5TOps())
>        CallOpc = ARMISD::CALL_NOLINK;
>      else if (doesNotRet && isDirect && !isARMFunc &&
> -             Subtarget->hasRAS() && !Subtarget->isThumb1Only())
> +             Subtarget->hasRAS() && !Subtarget->isThumb1Only() &&
> +	     // Emit regular call when code size is the priority
> +	     !FnAttributes.hasAttribute(Attributes::ForceSizeOpt))

Then this simplifies to just reference the bool.


>        // "mov lr, pc; b _foo" to avoid confusing the RSP
>        CallOpc = ARMISD::CALL_NOLINK;
>      else
> @@ -1606,7 +1609,9 @@
>    } else {
>      if (!isDirect && !Subtarget->hasV5TOps()) {
>        CallOpc = ARMISD::CALL_NOLINK;
> -    } else if (doesNotRet && isDirect && Subtarget->hasRAS())
> +    } else if (doesNotRet && isDirect && Subtarget->hasRAS() &&
> +	       // Emit regular call when code size is the priority
> +	       !FnAttributes.hasAttribute(Attributes::ForceSizeOpt))
>        // "mov lr, pc; b _foo" to avoid confusing the RSP
>        CallOpc = ARMISD::CALL_NOLINK;
>      else
> Index: test/CodeGen/ARM/call-noret-forsize.ll
> ===================================================================
> --- test/CodeGen/ARM/call-noret-forsize.ll	(revision 0)
> +++ test/CodeGen/ARM/call-noret-forsize.ll	(revision 0)
> @@ -0,0 +1,34 @@
> +; RUN: llc < %s -mtriple=armv7-apple-ios -mcpu=cortex-a8   | FileCheck %s -check-prefix=ARM
> +; RUN: llc < %s -mtriple=armv7-apple-ios -mcpu=swift       | FileCheck %s -check-prefix=SWIFT
> +; RUN: llc < %s -mtriple=thumbv7-apple-ios -mcpu=cortex-a8 | FileCheck %s -check-prefix=T2
> +; rdar://12348580
> +
> +define void @t1() noreturn forcesizeopt nounwind ssp {
> +entry:
> +; ARM: t1:
> +; ARM: bl _bar
> +
> +; SWIFT: t1:
> +; SWIFT: bl _bar
> +
> +; T2: t1:
> +; T2: blx _bar
> +  tail call void @bar() noreturn nounwind
> +  unreachable
> +}
> +
> +define void @t2() noreturn forcesizeopt nounwind ssp {
> +entry:
> +; ARM: t2:
> +; ARM: bl _t1
> +
> +; SWIFT: t2:
> +; SWIFT: bl _t1
> +
> +; T2: t2:
> +; T2: bl _t1
> +  tail call void @t1() noreturn nounwind
> +  unreachable
> +}
> +
> +declare void @bar() noreturn

On Oct 26, 2012, at 4:39 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> Hi,
> 
> Attached a patch to generate a cheaper sequence of code for calls on ARM when the code size is the main concern.
> 
> Quentin
> 
> <ARMISelLowering.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits