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

[Easwaran Raman via llvm-commits]

David is right about the root cause which results in negative threshold.
Prior to r252595, that was not an issue because it returned (Cost <
Threshold) and negative threshold with zero cost still produced the desired
effect.

I'll send out a patch that'll maintain thresholds always non-negative

- Easwaran


On Wed, Feb 24, 2016 at 12:10 AM, Xinliang David Li <xinliangli at gmail.com>
wrote:

> There is a problem in threshold adjust code. It is speculatively bumped up
> with a delta from the base value, and then for unreachable case, it is
> reset to 0 (should really be set to threshold - base_threshold). Later when
> the speculatively added value is subtracted again, inconsistent result may
> occur (i.e., returns can be inlined for a case where early bailout happens).
>
> David
>
> On Tue, Feb 23, 2016 at 10:46 PM, Akira Hatanaka via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> If Cost=0 is supposed to mean inlining has zero cost, we should make sure
>> Cost > 0 when the loop in InlineCost.cpp (line 1306) bails out early.
>>
>> On Tue, Feb 23, 2016 at 4:26 PM, Hans Wennborg <hans at chromium.org> wrote:
>>
>>> If Cost = 0 doesn't mean inlining the function has zero cost, that seems
>>> broken.
>>>
>>> I think eraman knows this code. Maybe he wants to take a look?
>>>
>>> On Mon, Feb 22, 2016 at 9:11 PM, Akira Hatanaka via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>> > Hi Hans,
>>> >
>>> > This commit (or r252595) causes the inliner to inline functions you
>>> don't
>>> > want to inline (large functions) in some cases. For example, if you
>>> compile
>>> > the attached test case with "opt -inline", function "callee" will
>>> always get
>>> > inlined even after replicating the inline-asm instruction in the last
>>> basic
>>> > block a large number of times. This happens because the loop in
>>> > CallAnalyzer::analyzeCall that visits the basic blocks of "callee"
>>> breaks
>>> > out when Cost exceeds Threshold, and when that happens Cost happens to
>>> be 0.
>>> > In this case Cost doesn't represent the true cost of the function
>>> (Cost<=0
>>> > doesn't mean "inlining is free").
>>> >
>>> > Could you take a look?
>>> >
>>> > 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
>>> >
>>>
>>
>>
>> _______________________________________________
>> 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/b09778a7/attachment.html>