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

[Quentin Colombet]

I have made the changes.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ARMISelLowering.patch
Type: application/octet-stream
Size: 2400 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121026/ff50cce7/attachment.obj>
-------------- next part --------------

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

> 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?
I found my answer :)
> 
>> 
>>>  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
>> 
>