[llvm-commits] [llvm] r165367 - /llvm/trunk/lib/Analysis/InlineCost.cpp

[Chandler Carruth]

On Sat, Oct 6, 2012 at 7:01 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Bob Wilson wrote:
>
>> Author: bwilson
>> Date: Sat Oct  6 20:11:19 2012
>> New Revision: 165367
>>
>> URL: http://llvm.org/viewvc/llvm-**project?rev=165367&view=rev<http://llvm.org/viewvc/llvm-project?rev=165367&view=rev>
>> Log:
>> Make sure always-inline functions get inlined.<rdar://problem/**12423986>
>>
>> Without this change, when the estimated cost for inlining a function with
>> an "alwaysinline" attribute was lower than the inlining threshold, the
>> getInlineCost function was returning that estimated cost rather than the
>> special InlineCost::AlwaysInlineCost value. That is fine in the normal
>> inlining case, but it can fail when the inliner considers the opportunity
>> cost of inlining into an internal or linkonce-odr function. It may decide
>> not to inline the always-inline function in that case. The fix here is
>> just
>> to make getInlineCost always return the special value for always-inline
>> functions. I ran into this building clang with libc++. Tablegen failed to
>> link because of an always-inline function that was not inlined. I have
>> been
>> unable to reduce the testcase down to a reasonable size.
>>
>
> I don't think this patch is correct. There's an -always-inline pass which
> shouldn't reach this code in InlineCost at all. Why didn't that kick in?
>

IIRC, we don't run that pass outside of -O0. See my email to Bob, I think
the better fix is to run that pass always, and remove all of th
ealways-inline hackery from this pass. See the silly hacks both in
PassManagerBuilder to allow clients to arbitrarily clobber the inliner pass
used, and in lib/CodeGen/BackendUtil in Clang to set this up.

Some of this stems from even more silliness with -O2 -fno-inline where we
still run the always inliner pass....


>
> Nick
>
>  Modified:
>>      llvm/trunk/lib/Analysis/**InlineCost.cpp
>>
>> Modified: llvm/trunk/lib/Analysis/**InlineCost.cpp
>> URL: http://llvm.org/viewvc/llvm-**project/llvm/trunk/lib/**
>> Analysis/InlineCost.cpp?rev=**165367&r1=165366&r2=165367&**view=diff<http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=165367&r1=165366&r2=165367&view=diff>
>> ==============================**==============================**
>> ==================
>> --- llvm/trunk/lib/Analysis/**InlineCost.cpp (original)
>> +++ llvm/trunk/lib/Analysis/**InlineCost.cpp Sat Oct  6 20:11:19 2012
>> @@ -142,6 +142,7 @@
>>
>>     int getThreshold() { return Threshold; }
>>     int getCost() { return Cost; }
>> +  bool isAlwaysInline() { return AlwaysInline; }
>>
>>     // Keep a bunch of stats about the cost savings found so we can print
>> them
>>     // out when debugging.
>> @@ -1057,7 +1058,8 @@
>>     // Check if there was a reason to force inlining or no inlining.
>>     if (!ShouldInline&&  CA.getCost()<  CA.getThreshold())
>>       return InlineCost::getNever();
>> -  if (ShouldInline&&  CA.getCost()>= CA.getThreshold())
>> +  if (ShouldInline&&  (CA.isAlwaysInline() ||
>>
>> +                       CA.getCost()>= CA.getThreshold()))
>>       return InlineCost::getAlways();
>>
>>     return llvm::InlineCost::get(CA.**getCost(), CA.getThreshold());
>>
>>
>> ______________________________**_________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/**mailman/listinfo/llvm-commits<http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>>
>>
> ______________________________**_________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/**mailman/listinfo/llvm-commits<http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121006/ef8b010c/attachment.html>