Replace hot-callsite based heuristic to use its own threshold parameter instead of share inline-hint parameter

[danielcdh]

Summary

Hot callsites should have higher threshold than inline hints. This patch uses separate threshold parameter for hot callsites.

[Prazek]

it looks good for me, but I am not really competent to accept it yet, so I guess wait for the other reviewers.

[davidxl]

should we have a fixed cutoff threshold, or make it adaptive to the hotness ? Or more generally, make it part of the global speedup analysis (the larger the global speedup, the larger the speedup) -- which we will soon have?

Another thing: PGO and autoFDO should reduce overall text size compared with O2 build, so we should also do the opposite -- if the caller has zero count (non samples with autoFDO), the threshold should be reduced.

[danielcdh]

add cold callsite heuristic

[danielcdh]

In D22368#488605, @davidxl wrote:

should we have a fixed cutoff threshold, or make it adaptive to the hotness ? Or more generally, make it part of the global speedup analysis (the larger the global speedup, the larger the speedup) -- which we will soon have?

I agree. But for now, the PSI interface only give use boolean value for hot/cold inquiry. How about we have this fixed cutoff in first, later when global speedup analysis is ready, we use the speedup as parameter to set the threshold?

Another thing: PGO and autoFDO should reduce overall text size compared with O2 build, so we should also do the opposite -- if the caller has zero count (non samples with autoFDO), the threshold should be reduced.

Added cold callsite heuristic.

[eraman]

lib/Analysis/InlineCost.cpp

69

Adding a threshold different from inlinehint-threshold makes sense. But if you want to set the default this high, then it is important to have numbers to justify. Running spec with different thresholds and a comment here saying you chose this default because this gives the best performance/size tradeoff is important.

[eraman]

LGTM.

[Copying Dehao's comments from the review thread for reference]

I experimented different thresholds from 325 (original) to 4000. The code size change is within 2% fall all speccpu2006 int benchmarks. In terms of performance. Up to 4% speedup is observed for perlbench when changing threshold from 325 to 3000. And above 3000, the performance curve remains flat. For all other speccpu2006 int benchmarks, the performance change does not escape noise range when changing threshold from 325 to 4000. Similar performance/size result is observed for internal benchmarks. So I think 3000 seems to be a sweet spot for the threshold.

[danielcdh]

This patch still needs an approval?

[eraman]

In D22368#501206, @danielcdh wrote:

This patch still needs an approval

LGTM from me. Don't know if David has any other comments.

lib/Analysis/InlineCost.cpp

 
 // We introduce this threshold to help performance of instrumentation based
 // PGO before we actually hook up inliner with analysis passes such as BPI and
 // BFI.
 static cl::opt<int> ColdThreshold(
     "inlinecold-threshold", cl::Hidden, cl::init(225),
     cl::desc("Threshold for inlining functions with cold attribute"));
 
+static cl::opt<int>
+    HotCallSiteThreshold("hot-callsite-threshold", cl::Hidden, cl::init(3000),
+                         cl::ZeroOrMore,
+                         cl::desc("Threshold for hot callsites "));
+
 namespace {
 
 class CallAnalyzer : public InstVisitor<CallAnalyzer, bool> {
   typedef InstVisitor<CallAnalyzer, bool> Base;
   friend class InstVisitor<CallAnalyzer, bool>;
 
   /// The TargetTransformInfo available for this compilation.
   const TargetTransformInfo &TTI;
if (DefaultInlineThreshold.getNumOccurrences() > 0) {
     // attributes when they would decrease the threshold.
     if (Caller->optForMinSize() && OptMinSizeThreshold < Threshold)
       Threshold = OptMinSizeThreshold;
     else if (Caller->optForSize() && OptSizeThreshold < Threshold)
       Threshold = OptSizeThreshold;
   }
 
   bool HotCallsite = false;
+  bool ColdCallsite = false;
   uint64_t TotalWeight;
-  if (CS.getInstruction()->extractProfTotalWeight(TotalWeight) &&
-      PSI->isHotCount(TotalWeight))
+  if (CS.getInstruction()->extractProfTotalWeight(TotalWeight))
+    if (PSI->isHotCount(TotalWeight))
       HotCallsite = true;
+    else if (PSI->isColdCount(TotalWeight))
+      ColdCallsite = true;
 
   // Listen to the inlinehint attribute or profile based hotness information
   // when it would increase the threshold and the caller does not need to
   // minimize its size.
   bool InlineHint = Callee.hasFnAttribute(Attribute::InlineHint) ||
-                    PSI->isHotFunction(&Callee) ||
-                    HotCallsite;
+                    PSI->isHotFunction(&Callee);
   if (InlineHint && HintThreshold > Threshold && !Caller->optForMinSize())
     Threshold = HintThreshold;
 
+  if (HotCallsite && HotCallSiteThreshold > Threshold &&
+      !Caller->optForMinSize())
+    Threshold = HotCallSiteThreshold;
+
   bool ColdCallee = PSI->isColdFunction(&Callee);
   // Command line argument for DefaultInlineThreshold will override the default
   // ColdThreshold. If we have -inline-threshold but no -inlinecold-threshold,
   // do not use the default cold threshold even if it is smaller.
   if ((DefaultInlineThreshold.getNumOccurrences() == 0 ||
        ColdThreshold.getNumOccurrences() > 0) &&
-      ColdCallee && ColdThreshold < Threshold)
+      (ColdCallee || ColdCallsite) && ColdThreshold < Threshold)
     Threshold = ColdThreshold;
 
   // Finally, take the target-specific inlining threshold multiplier into
   // account.
   Threshold *= TTI.getInliningThresholdMultiplier();
 }
 
 bool CallAnalyzer::visitCmpInst(CmpInst &I) {

test/Transforms/Inline/inline-hot-callsite.ll

-; RUN: opt < %s -inline -inline-threshold=0 -inlinehint-threshold=100 -S | FileCheck %s
+; RUN: opt < %s -inline -inline-threshold=0 -hot-callsite-threshold=100 -S | FileCheck %s
 
 ; This tests that a hot callsite gets the (higher) inlinehint-threshold even without
 ; without inline hints and gets inlined because the cost is less than
 ; inlinehint-threshold. A cold callee with identical body does not get inlined because
 ; cost exceeds the inline-threshold
 
 define i32 @callee1(i32 %x) {
   %x1 = add i32 %x, 1