[CSSPGO] Use preinliner decision by default when available

[wenlei]

Summary

For CSSPGO, turn on sample-profile-use-preinliner by default. This simplifies the use of llvm-profgen preinliner as it's now simply driven by ContextShouldBeInlined flag for each context profile without needing extra compiler switch.

Note that llvm-profgen's preinliner is still off by default, under switch csspgo-preinliner.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

Will non-inlined context be merged into the base profile by the preinliner? Wondering if we can have contexts without ContextShouldBeInlined in the profile after processed by the preinliner.

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

Not inlined context will all be merged currently. See the comment I added above.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

So we'll not see the context profile here? Can such context only be seen when the preinliner is off?

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

Correct. When preinliner is on in llvm-profgen, we will only see context profile if preinliner decides to inline. When preinliner is off, we will see all context profiles.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

Thanks for the confirmation. Can you update the comment to reflect that?

[wenlei]

update test

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

The comment mentions that:

Note that we don't need to handle negative decision from preinliner as context profile for not inlined calls are merged by preinliner already.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

Oh, I actually meant to say there shouldn't be a negative decision from preinliner, since Candidate.CalleeSamples would be null in that case. I might misunderstand your comment.

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

I actually meant to say there shouldn't be a negative decision from preinliner, since Candidate.CalleeSamples would be null in that case.

By decision from preinliner, it literally means inline decision made by preinliner in llvm-profgen. Preinliner would reject inlining for some calls, and these are the negative decision from preinliner.

Candidate.CalleeSamples will never be null. If preinliner makes a negative inline decision, the corresponding context profile won't exist, and we won't see the callee as inline candidate (see getInlineCandidate). That's why we don't need to handle negative decisions explicitly.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1303

Sounds good.

llvm/lib/Transforms/IPO/SampleProfile.cpp

SampleProfileLoader::shouldInlineCandidate(InlineCandidate &Candidate) {
 
   // With CSSPGO, the preinliner in llvm-profgen can estimate global inline
   // decisions based on hotness as well as accurate function byte sizes for
   // given context using function/inlinee sizes from previous build. It
   // stores the decision in profile, and also adjust/merge context profile
   // aiming at better context-sensitive post-inline profile quality, assuming
   // all inline decision estimates are going to be honored by compiler. Here
   // we replay that inline decision under `sample-profile-use-preinliner`.
-  if (UsePreInlinerDecision) {
-    if (Candidate.CalleeSamples->getContext().hasAttribute(
-            ContextShouldBeInlined))
-      return InlineCost::getAlways("preinliner");
-    else
-      return InlineCost::getNever("preinliner");
-  }
+  // Note that we don't need to handle negative decision from preinliner as
+  // context profile for not inlined calls are merged by preinliner already.
+  if (UsePreInlinerDecision &&
+      Candidate.CalleeSamples->getContext().hasAttribute(
 
   // For old FDO inliner, we inline the call site as long as cost is not
   // "Never". The cost-benefit check is done earlier.
   if (!CallsitePrioritizedInline) {
     return InlineCost::get(Cost.getCost(), INT_MAX);
   }
 
   // Otherwise only use the cost from call analyzer, but overwite threshold with
if (Reader->profileIsCS()) {
     FunctionSamples::ProfileIsCS = true;
 
     // 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;
+
     // Enable iterative-BFI by default for CSSPGO.
     if (!UseIterativeBFIInference.getNumOccurrences())
       UseIterativeBFIInference = true;
 
     // Tracker for profiles under different context
     ContextTracker = std::make_unique<SampleContextTracker>(
         Reader->getProfiles(), &GUIDToFuncNameMap);
   }

llvm/test/Transforms/SampleProfile/csspgo-use-preinliner.ll

 ; Test for CSSPGO's sample loader inlining to make sure it can use llvm-profgen preinliner's decision
 
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/csspgo-use-preinliner.prof -pass-remarks=inline -sample-profile-prioritized-inline -profile-sample-accurate -sample-profile-use-preinliner=0 -S 2>&1 | FileCheck %s --check-prefix=DEFAULT
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/csspgo-use-preinliner.prof -pass-remarks=inline -sample-profile-prioritized-inline -profile-sample-accurate -sample-profile-use-preinliner=1 -S 2>&1 | FileCheck %s --check-prefix=PREINLINE
 
 
 ; DEFAULT: '_Z5funcAi' inlined into 'main'
 ; DEFAULT-NOT: inlined into
 
 ; PREINLINE-NOT: inlined into
+; PREINLINE: '_Z5funcAi' inlined into 'main'
+; PREINLINE: '_Z8funcLeafi' inlined into 'main'
 ; PREINLINE: '_Z8funcLeafi' inlined into '_Z5funcBi'
-; PREINLINE: '_Z8funcLeafi' inlined into '_Z5funcAi'
 ; PREINLINE-NOT: inlined into
 
 @factor = dso_local global i32 3, align 4, !dbg !0
 
 define dso_local i32 @main() local_unnamed_addr #0 !dbg !18 {
 entry:
   br label %for.body, !dbg !25