[llvm] r259915 - CallAnalyzer::analyzeCall: change the condition back to "Cost < Threshold"

[Xinliang David Li via llvm-commits]

As Akira's case shows, the following can happen:

1) under certain conditions (unreachable path etc), the threshold was
adjusted to 0
2) in analyzeCall, in the worklist loop, the threshold can further be
adjusted to negative number (single BB bonus)
3) in AnalyzeBlock, it bails out early when cost == 0 -- even the real cost
of the function is much higher
4) outside the worklist loop, with Hans's change, the 'zero' cost is
compared with the max(0, threshold), and the result will be true.

David

On Wed, Feb 24, 2016 at 4:19 PM, Easwaran Raman via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> I'm curious about the cause of the size regression. The effect of  r252595
> is two-fold:
>
> 1. Inline zero-cost callees even if the threshold is negative or 0.
> 2. Inline positive-cost callees when they equal to the threshold (as
> opposed to less than the threshold).
>
> This change only reverts the effect  2 above. If that fixes the size
> regression, is it the case that a lot of callees had cost equal to
> threshold and the inlining decision got reversed? That could happen of
> course, but I doubt that should cause a significant regression. I suspect
> there are other subtle bugs in the code, so looking at specific instances
> will be useful.
>
> - Easwaran
>
>
> On Fri, Feb 5, 2016 at 12:32 PM, Hans Wennborg via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: hans
>> Date: Fri Feb  5 14:32:42 2016
>> New Revision: 259915
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=259915&view=rev
>> Log:
>> CallAnalyzer::analyzeCall: change the condition back to "Cost < Threshold"
>>
>> In r252595, I inadvertently changed the condition to "Cost <= Threshold",
>> which caused a significant size regression in Chrome. This commit
>> rectifies
>> that.
>>
>> 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=259915&r1=259914&r2=259915&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/InlineCost.cpp (original)
>> +++ llvm/trunk/lib/Analysis/InlineCost.cpp Fri Feb  5 14:32:42 2016
>> @@ -1392,7 +1392,7 @@ bool CallAnalyzer::analyzeCall(CallSite
>>    else if (NumVectorInstructions <= NumInstructions / 2)
>>      Threshold -= (FiftyPercentVectorBonus - TenPercentVectorBonus);
>>
>> -  return Cost <= std::max(0, Threshold);
>> +  return Cost < std::max(1, Threshold);
>>  }
>>
>>  #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
>>
>>
>> _______________________________________________
>> 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/20160224/817c3f4d/attachment.html>