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

[Xinliang David Li via llvm-commits]

-  return Cost <= std::max(0, Threshold);
+  return Cost < std::max(1, Threshold);

Is the change 0 -> 1 has any effect on size? Or the size regression is
fixed only by changing <= to <?

David

On Wed, Feb 24, 2016 at 4:40 PM, Hans Wennborg via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Yes, this change only reverts 2), and that did fix the size regression.
>
> I don't have any binaries to compare, unfortunately, but since the
> only thing my change did was to effectively move the threshold by 1,
> the only explanation I can think off is that some number of functions
> were on the threshold and this affected the inlining decision.
>
> The significant size increase I saw was 150 KB (on a ~50 MB binary).
> What's the typical range of Cost and Threshold?
>
> 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
> >
> _______________________________________________
> 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/20160225/019ea315/attachment.html>