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

[Quentin Colombet]

Hi Jim,

On Oct 26, 2012, at 4:58 PM, Jim Grosbach <grosbach at apple.com> wrote:

> 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);

Doing that I reach the 80 characters limit for a line.
What is the current accepted way of splitting this kind of long chain of method calls?

> 
>>   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
>