Use the inlinehint-threshold for hot callees.

[eraman]

Summary

This patch uses the higher inlinehint-threshold for callees that are considered hot. The frontend already uses the same hotness threshold (callee's entry count >= 30% of max function entry count) to set inline hints on such callees, which results in the inlinehint-threshold being used anyway, so there is no functional change in this patch. The idea is to remove that code that sets inline hints based on hotness from the frontend and make the PGO based inlining decisions in the inliner itself. Eventually what we really want is callsite based hotness thresholds, but that is a heuristic change that needs tuning and will be done in a separate patch.

[davidxl]

The title of the patch is wrong. It is not setting higher threshold but moving (part of it) the FE Inline hint logic to the inliner.

[davidxl]

The clean up looks ok to me (NFC at this point). I added Duncan to the list to see if there are any concerns.

[eraman]

Well, it does use a higher (when compared to the default) threshold, but
I've changed the title and description to make it more clear.

[dnovillo]

Once this is in, please update lib/Transforms/IPO/SampleProfile.cpp. It does something similar.

Also, should cold functions be marked with the cold hint?

[davidxl]

LGTM. As Justin says, please continue the clean up effort: 1) unify the profile summary data for PGO and Sample FDO 2) move the cold attribute logic to LLVM

[eraman]

1. Use the hint threshold only if it is greater than regular threshold.
2. Added a test case.

[eraman]

Duncan and Justin, does this patch look good?

[eraman]

I am now checking for cold calls and using the cold threshold as well and added a test case.

Earlier in this thread I had commented that Cold attribute is used in other places as well, but I was wrong. Branch probability computation checks for Cold attribute in instruction and not the function. With this change, both setting of cold and inlinehint attribute can be removed from CodeGenPGO.cpp in CFE. Does this look good?

[davidxl]

This looks very good to me (also addressed cold attribute feedback by Justin) -- and the test cases are nice. Ok to check in with the 'nits' fixed. Looking forward to the FE cleanup.

lib/Transforms/IPO/Inliner.cpp

316 ↗

(On Diff #43282)

I would suggest parametize 0.3 and 0.01 below, but since there is on-going work on profile summary, it is probably not necessary.

test/Transforms/Inline/inline-cold-callee.ll

36 ↗

(On Diff #43282)

nit -- please make MaxFunctionCount larger so that the test case is more stable to future tunings.

[eraman]

I'll address David's comment on test case (although I don't think it matters much) and if I don't hear from Justin and Duncan will submit this patch later today.

llvm/trunk/lib/Transforms/IPO/Inliner.cpp

unsigned Inliner::getInlineThreshold(CallSite CS) const {
   Function *Caller = CS.getCaller();
   bool OptSize = Caller && !Caller->isDeclaration() &&
                  // FIXME: Use Function::optForSize().
                  Caller->hasFnAttribute(Attribute::OptimizeForSize);
   if (!(InlineLimit.getNumOccurrences() > 0) && OptSize &&
       OptSizeThreshold < Threshold)
     Threshold = OptSizeThreshold;
 
-  // Listen to the inlinehint attribute when it would increase the threshold
-  // and the caller does not need to minimize its size.
   Function *Callee = CS.getCalledFunction();
-  bool InlineHint = Callee && !Callee->isDeclaration() &&
-                    Callee->hasFnAttribute(Attribute::InlineHint);
+  if (!Callee || Callee->isDeclaration())
+    return Threshold;
+
+  // If profile information is available, use that to adjust threshold of hot
+  // and cold functions.
+  // FIXME: The heuristic used below for determining hotness and coldness are
+  // based on preliminary SPEC tuning and may not be optimal. Replace this with
+  // a well-tuned heuristic based on *callsite* hotness and not callee hotness.
+  uint64_t FunctionCount = 0, MaxFunctionCount = 0;
+  bool HasPGOCounts = false;
+  if (Callee->getEntryCount() &&
+      Callee->getParent()->getMaximumFunctionCount()) {
+    HasPGOCounts = true;
+    FunctionCount = Callee->getEntryCount().getValue();
+    MaxFunctionCount =
+        Callee->getParent()->getMaximumFunctionCount().getValue();
+  }
+
+  // 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) ||
+      (HasPGOCounts &&
+       FunctionCount >= (uint64_t)(0.3 * (double)MaxFunctionCount));
   if (InlineHint && HintThreshold > Threshold &&
       !Caller->hasFnAttribute(Attribute::MinSize))
     Threshold = HintThreshold;
 
-  // Listen to the cold attribute when it would decrease the threshold.
-  bool ColdCallee = Callee && !Callee->isDeclaration() &&
-                    Callee->hasFnAttribute(Attribute::Cold);
+  // Listen to the cold attribute or profile based coldness information
+  // when it would decrease the threshold.
+  bool ColdCallee =
+      Callee->hasFnAttribute(Attribute::Cold) ||
+      (HasPGOCounts &&
+       FunctionCount <= (uint64_t)(0.01 * (double)MaxFunctionCount));
   // Command line argument for InlineLimit 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 ((InlineLimit.getNumOccurrences() == 0 ||
        ColdThreshold.getNumOccurrences() > 0) && ColdCallee &&
       ColdThreshold < Threshold)
     Threshold = ColdThreshold;
 

llvm/trunk/test/Transforms/Inline/inline-cold-callee.ll

+; RUN: opt < %s -inline  -inlinecold-threshold=0 -S | FileCheck %s
+
+; This tests that a cold callee gets the (lower) inlinecold-threshold even without
+; Cold hint and does not get inlined because the cost exceeds the inlinecold-threshold.
+; A callee with identical body does gets inlined because cost fits within the
+; inline-threshold
+
+define i32 @callee1(i32 %x) !prof !1 {
+  %x1 = add i32 %x, 1
+  %x2 = add i32 %x1, 1
+  %x3 = add i32 %x2, 1
+
+  ret i32 %x3
+}
+
+define i32 @callee2(i32 %x) !prof !2 {
+; CHECK-LABEL: @callee2(
+  %x1 = add i32 %x, 1
+  %x2 = add i32 %x1, 1
+  %x3 = add i32 %x2, 1
+
+  ret i32 %x3
+}
+
+define i32 @caller2(i32 %y1) !prof !2 {
+; CHECK-LABEL: @caller2(
+; CHECK: call i32 @callee2
+; CHECK-NOT: call i32 @callee1
+; CHECK: ret i32 %x3.i
+  %y2 = call i32 @callee2(i32 %y1)
+  %y3 = call i32 @callee1(i32 %y2)
+  ret i32 %y3
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MaxFunctionCount", i32 1000}
+!1 = !{!"function_entry_count", i64 100}
+!2 = !{!"function_entry_count", i64 1}
+

llvm/trunk/test/Transforms/Inline/inline-hot-callee.ll

+; RUN: opt < %s -inline -inline-threshold=0 -inlinehint-threshold=100 -S | FileCheck %s
+
+; This tests that a hot callee gets the (higher) inlinehint-threshold even 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) !prof !1 {
+  %x1 = add i32 %x, 1
+  %x2 = add i32 %x1, 1
+  %x3 = add i32 %x2, 1
+
+  ret i32 %x3
+}
+
+define i32 @callee2(i32 %x) !prof !2 {
+; CHECK-LABEL: @callee2(
+  %x1 = add i32 %x, 1
+  %x2 = add i32 %x1, 1
+  %x3 = add i32 %x2, 1
+
+  ret i32 %x3
+}
+
+define i32 @caller2(i32 %y1) !prof !2 {
+; CHECK-LABEL: @caller2(
+; CHECK: call i32 @callee2
+; CHECK-NOT: call i32 @callee1
+; CHECK: ret i32 %x3.i
+  %y2 = call i32 @callee2(i32 %y1)
+  %y3 = call i32 @callee1(i32 %y2)
+  ret i32 %y3
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"MaxFunctionCount", i32 10}
+!1 = !{!"function_entry_count", i64 10}
+!2 = !{!"function_entry_count", i64 1}
+