[Inlining] Set inline-deferral-scale to 2.

[kazu]

Summary

This patch sets inline-deferral-scale to 2.

Both internal and SPEC benchmarking show that 2 is the best number
among -1, 2, 3, and 4.

inline-deferral-scale SPECint2006

-1  38.0 (the default without this patch)
 2  38.5
 3  38.1
 4  38.1

With the new default number, shouldBeDeferred returns true if:

TotalCost < IC.getCost() * 2

where

TotalCost is TotalSecondaryCost + IC.getCost() * NumCallerUsers.

If TotalCost >= 0 and NumCallerUsers >= 2, then
TotalCost >= IC.getCost() * 2, so shouldBeDeferred returns true only
when NumCallerUsers is 1.

Now, if TotalSecondaryCost < 0, which can happen if
InlineConstants::LastCallToStaticBonus, a huge number, has been
subtracted from TotalSecondaryCost, then TotalCost may be negative.
In this case, shouldBeDeferred may return true even when
NumCallerUsers >= 2.

[xbolva00]

SPEC numbers look great:)

[kazu]

PTAL Thanks!

[davidxl]

How does it affect text size overall?

[kazu]

In D80229#2048327, @davidxl wrote:

How does it affect text size overall?

Thanks for a review!

PGO+ThinLTO bootstrap of clang results in smaller .text sections:

Section	Before	After	Change
.text.hot	5920586	5917770	-0.05%
.text	7155209	7085897	-0.97%
.text.unlikely	35922640	35625712	-0.83%
.text.startup	728048	728032	-0.00%

[davidxl]

thanks for the data. Would it be possible to collect size data with plain O2 or O3 build?

[nikic]

Here are some text size numbers from CTMark for this patch: http://llvm-compile-time-tracker.com/compare.php?from=361e4f14e35981e7498a38ab7c31da7fb7414fb8&to=d9666f0e34439c226438f1ad0e64a86456c323db&stat=size-text There is a huge (4-6%) text size reduction on sqlite. Don't know if that's a good sign or a bad sign in this case...

[davidxl]

performance win + size reduction is double win. LGTM

[lebedev.ri]

It's kinda off-topic and not sure if anyone is interested in more test cases,
but i have a codebase for which the current inlining decisions still seem
just a little bit too weak. I.e. if i drop all __attribute__((always_inline)) attrs,
i see +10% perf drops in all of those cases. Inline parameters isn't something i looked into,
so i'm not really sure where to look. Feel free to let me know if anyone is interested.

[nikic]

I finally understood what this code is doing ... and I'm wondering why you only tested scales from 2 upwards, and did not test a scale of 1. Scale 1 should be the one that is actually "correct" in terms of modeling, as we are choosing to incur TotalCost in place of IC.getCost().

llvm/lib/Analysis/InlineAdvisor.cpp

InlineRemarkAttribute("inline-remark-attribute", cl::init(false),
                                    " callsites processed by inliner but decided"
                                    " to be not inlined"));
 
 // An integer used to limit the cost of inline deferral.  The default negative
 // number tells shouldBeDeferred to only take the secondary cost into account.
 static cl::opt<int>
     InlineDeferralScale("inline-deferral-scale",
                         cl::desc("Scale to limit the cost of inline deferral"),
-                        cl::init(-1), cl::Hidden);
+                        cl::init(2), cl::Hidden);
 
 namespace {
 class DefaultInlineAdvice : public InlineAdvice {
 public:
   DefaultInlineAdvice(DefaultInlineAdvisor *Advisor, CallBase &CB,
                       Optional<InlineCost> OIC, OptimizationRemarkEmitter &ORE)
       : InlineAdvice(Advisor, CB, OIC.hasValue()), OriginalCB(&CB), OIC(OIC),
         ORE(ORE), DLoc(CB.getDebugLoc()), Block(CB.getParent()) {}