[PATCH] D124632: [CSSPGO] Turn on priority inlining for probe-only profile

[Hongtao Yu via Phabricator via llvm-commits]

hoy created this revision.
Herald added subscribers: ormris, modimo, wenlei, hiraditya.
Herald added a project: All.
hoy requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

We have seen that the prioirty inliner delivered on-par performance with the old inliner for probe-only CSSPGO profile, as long as without a size budget. I'm turning on the priority inliner for probe-only profile by default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124632

Files:
  llvm/lib/Transforms/IPO/SampleProfile.cpp


Index: llvm/lib/Transforms/IPO/SampleProfile.cpp
===================================================================
--- llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -2016,31 +2016,36 @@
       SampleProfileUseProfi = true;
     if (!EnableExtTspBlockPlacement.getNumOccurrences())
       EnableExtTspBlockPlacement = true;
-  }
-
-  if (Reader->profileIsCS() || Reader->profileIsPreInlined()) {
-    ProfileIsCS = Reader->profileIsCS();
     // Enable priority-base inliner and size inline by default for CSSPGO.
     if (!ProfileSizeInline.getNumOccurrences())
       ProfileSizeInline = true;
     if (!CallsitePrioritizedInline.getNumOccurrences())
       CallsitePrioritizedInline = true;
-
-    // For CSSPGO, use preinliner decision by default when available.
-    if (!UsePreInlinerDecision.getNumOccurrences())
-      UsePreInlinerDecision = true;
-
     // For CSSPGO, we also allow recursive inline to best use context profile.
     if (!AllowRecursiveInline.getNumOccurrences())
       AllowRecursiveInline = true;
 
-    if (FunctionSamples::ProfileIsCS) {
-      // Tracker for profiles under different context
-      ContextTracker = std::make_unique<SampleContextTracker>(
-          Reader->getProfiles(), &GUIDToFuncNameMap);
+    if (Reader->profileIsPreInlined()) {
+      if (!UsePreInlinerDecision.getNumOccurrences())
+        UsePreInlinerDecision = true;
+    } else if (!Reader->profileIsCS()) {
+      // Non-CS profile should be fine without a function size budget for the
+      // inliner since the contexts in the profile are all from inlining in
+      // the prevoius build, thus they are bounded.
+      if (!ProfileInlineLimitMin.getNumOccurrences())
+        ProfileInlineLimitMin = std::numeric_limits<unsigned>::max();
+      if (!ProfileInlineLimitMax.getNumOccurrences())
+        ProfileInlineLimitMax = std::numeric_limits<unsigned>::max();
     }
   }
 
+  if (FunctionSamples::ProfileIsCS) {
+    ProfileIsCS = Reader->profileIsCS();
+    // Tracker for profiles under different context
+    ContextTracker = std::make_unique<SampleContextTracker>(
+        Reader->getProfiles(), &GUIDToFuncNameMap);
+  }
+
   // Load pseudo probe descriptors for probe-based function samples.
   if (Reader->profileIsProbeBased()) {
     ProbeManager = std::make_unique<PseudoProbeManager>(M);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124632.425877.patch
Type: text/x-patch
Size: 2370 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220428/26cd294e/attachment.bin>