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

[David Blaikie via llvm-commits]

On Mon, Jun 12, 2017 at 6:34 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

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

Ah, thanks!

r305267


>
>
>>
>> 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/5feea0e6/attachment.html>