[llvm] r305245 - Inliner: Don't remove calls to readnone+nounwind (but not always_inline) functions in the AlwaysInliner

[Chandler Carruth via llvm-commits]

On Mon, Jun 12, 2017 at 4:04 PM David Blaikie via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Hey Chandler - wouldn't mind a touch of post-commit review here (just in
> case this wouldn't've otherwise crossed your dashboard).
>
> Specifically this moves the shouldInline call in the non-trivial case to
> before the InlineHistory lookup. I'm wondering if that could have a
> significant performance impact such that it'd be worth consulting the
> InlineHistory before the shouldInline test.
>
> (similarly, I don't have a great sense for whether shouldInline might be
> substantially more expensive than 'isInstructionTriviallyDead' in case it'd
> be better for those to be in the opposite order)
>

shouldInline is very, very expensive. Please check the inline history
first, and check for anything like trivially dead instructions first.


>
> This is currently the tidiest way to write this - but if those orderings
> are likely to be important I'm happy to refactor it to address them.
>
> - Dave
>
>
> On Mon, Jun 12, 2017 at 4:01 PM David Blaikie via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: dblaikie
>> Date: Mon Jun 12 18:01:17 2017
>> New Revision: 305245
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=305245&view=rev
>> Log:
>> Inliner: Don't remove calls to readnone+nounwind (but not always_inline)
>> functions in the AlwaysInliner
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/IPO/Inliner.cpp
>>     llvm/trunk/test/Transforms/Inline/always-inline.ll
>>
>> Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=305245&r1=305244&r2=305245&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Mon Jun 12 18:01:17 2017
>> @@ -523,6 +523,16 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG
>>        if (!Callee || Callee->isDeclaration())
>>          continue;
>>
>> +      // FIXME for new PM: because of the old PM we currently generate
>> ORE and
>> +      // in turn BFI on demand.  With the new PM, the ORE dependency
>> should
>> +      // just become a regular analysis dependency.
>> +      OptimizationRemarkEmitter ORE(Caller);
>> +
>> +      // If the policy determines that we should inline this function,
>> +      // delete the call instead.
>> +      if (!shouldInline(CS, GetInlineCost, ORE))
>> +        continue;
>> +
>>        // If this call site is dead and it is to a readonly function, we
>> should
>>        // just delete the call instead of trying to inline it, regardless
>> of
>>        // size.  This happens because IPSCCP propagates the result out of
>> the
>> @@ -548,15 +558,6 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG
>>          // Get DebugLoc to report. CS will be invalid after Inliner.
>>          DebugLoc DLoc = CS.getInstruction()->getDebugLoc();
>>          BasicBlock *Block = CS.getParent();
>> -        // FIXME for new PM: because of the old PM we currently generate
>> ORE and
>> -        // in turn BFI on demand.  With the new PM, the ORE dependency
>> should
>> -        // just become a regular analysis dependency.
>> -        OptimizationRemarkEmitter ORE(Caller);
>> -
>> -        // If the policy determines that we should inline this function,
>> -        // try to do so.
>> -        if (!shouldInline(CS, GetInlineCost, ORE))
>> -          continue;
>>
>>          // Attempt to inline the function.
>>          using namespace ore;
>>
>> Modified: llvm/trunk/test/Transforms/Inline/always-inline.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/always-inline.ll?rev=305245&r1=305244&r2=305245&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/Inline/always-inline.ll (original)
>> +++ llvm/trunk/test/Transforms/Inline/always-inline.ll Mon Jun 12
>> 18:01:17 2017
>> @@ -305,3 +305,14 @@ entry:
>>    ret void
>>  ; CHECK: ret void
>>  }
>> +
>> +define void @inner14() readnone nounwind {
>> +; CHECK: define void @inner14
>> +  ret void
>> +}
>> +
>> +define void @outer14() {
>> +; CHECK: call void @inner14
>> +  call void @inner14()
>> +  ret void
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170613/5b73d76d/attachment.html>