patch for functions with indirect calls and always_inline attribute

[Gerolf Hoflehner]

Please see below.

On Apr 14, 2014, at 2:56 PM, Eric Christopher <echristo at gmail.com> wrote:

> I could be down for this...
> 
> Do you have any numbers?
No. I ran into this when creating a test case for another bug fix. 
> 
> Nits on the patch:
> 
>     // Disallow inlining of functions which contain an indirect branch.
> -    if (isa<IndirectBrInst>(BI->getTerminator()))
> +    if (isa<IndirectBrInst>(BI->getTerminator()) &&
> +        !F.hasFnAttribute(Attribute::AlwaysInline))
> 
> Update the comment.
Ok.
> I'm assuming this can't be hoisted because of the
> returns_twice aspect yes?

Yes, for now I only wanted to address indirect branches and leave
remaining checks as they are.

> 
> -define i32 @inner5(i8* %addr) alwaysinline {
> +define i32 @inner5(i8* %addr) {
> 
> Why the change?
This is to keep the old test behavior which relied on *not* inlining inner5 because of an indirect branch.
> 
> -eric
> 
> On Mon, Apr 14, 2014 at 2:49 PM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
>> Hi
>> 
>> llvm currently does not inline functions with indirect branches. We had some
>> internal discussion and think the always_inline attribute should overrule
>> that restriction.
>> 
>> Gerolf
>> 
>> 
>> 
>> 
>> 
>> Index: lib/Analysis/IPA/InlineCost.cpp
>> ===================================================================
>> --- lib/Analysis/IPA/InlineCost.cpp     (revision 206204)
>> +++ lib/Analysis/IPA/InlineCost.cpp     (working copy)
>> @@ -1301,7 +1301,8 @@
>>                                    Attribute::ReturnsTwice);
>>   for (Function::iterator BI = F.begin(), BE = F.end(); BI != BE; ++BI) {
>>     // Disallow inlining of functions which contain an indirect branch.
>> -    if (isa<IndirectBrInst>(BI->getTerminator()))
>> +    if (isa<IndirectBrInst>(BI->getTerminator()) &&
>> +        !F.hasFnAttribute(Attribute::AlwaysInline))
>>       return false;
>> 
>> 
>> 
>>     for (BasicBlock::iterator II = BI->begin(), IE = BI->end(); II != IE;
>> Index: test/Transforms/Inline/always-inline-attribute.ll
>> ===================================================================
>> --- test/Transforms/Inline/always-inline-attribute.ll   (revision 0)
>> +++ test/Transforms/Inline/always-inline-attribute.ll   (working copy)
>> @@ -0,0 +1,26 @@
>> +; RUN: opt < %s -O3 -S | FileCheck %s
>> + at gv = external global i32
>> +
>> +define i32 @main() nounwind {
>> +; CHECK-NOT: call i32 @foo
>> +  %1 = load i32* @gv, align 4
>> +  %2 = tail call i32 @foo(i32 %1)
>> +  unreachable
>> +}
>> +
>> +define internal i32 @foo(i32) alwaysinline {
>> +  br label %2
>> +
>> +; <label>:2                                       ; preds = %8, %1
>> +  %3 = phi i32 [ %0, %1 ], [ %10, %8 ]
>> +  %4 = phi i8* [ blockaddress(@foo, %2), %1 ], [ %6, %8 ]
>> +  %5 = icmp eq i32 %3, 1
>> +  %6 = select i1 %5, i8* blockaddress(@foo, %8), i8* %4
>> +  %7 = add nsw i32 %3, -1
>> +  br label %8
>> +
>> +; <label>:8                                       ; preds = %8, %2
>> +  %9 = phi i32 [ %7, %2 ], [ %10, %8 ]
>> +  %10 = add nsw i32 %9, -1
>> +  indirectbr i8* %6, [label %2, label %8]
>> +}
>> Index: test/Transforms/Inline/always-inline.ll
>> ===================================================================
>> --- test/Transforms/Inline/always-inline.ll     (revision 206203)
>> +++ test/Transforms/Inline/always-inline.ll     (working copy)
>> @@ -78,7 +78,7 @@
>>   ret i32 %add
>> }
>> 
>> 
>> 
>> -define i32 @inner5(i8* %addr) alwaysinline {
>> +define i32 @inner5(i8* %addr) {
>> entry:
>>   indirectbr i8* %addr, [ label %one, label %two ]
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>