[CSSPGO] Turn on profi and ext-tsp when using probe-based profile.

[hoy]

Summary

Probe-based profile leads to a better performance when combined with profi and ext-tsp block layout. I'm turning them on by default.

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

2036–2043

I would structure it slightly differently..

if (Reader->profileIsCSFlat() || Reader->profileIsCSNested() || Reader->profileIsProbeBased()) {
    // Enable iterative-BFI by default for CSSPGO.
    if (!UseIterativeBFIInference.getNumOccurrences())
      UseIterativeBFIInference = true;
    // Enable Profi by default for CSSPGO.
    if (!SampleProfileUseProfi.getNumOccurrences())
      SampleProfileUseProfi = true;

    // Enable EXT-TSP block layout for CSSPGO.
    if (!EnableExtTspBlockPlacement.getNumOccurrences())
      EnableExtTspBlockPlacement = true;
}
else if (Reader->profileIsCSNested() || Reader->profileIsProbeBased()) {
  ...
}

[hoy]

Updating D122442: [CSSPGO] Turn on profi and ext-tsp when using probe-based profile.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

2036–2043

Sounds good without the else.

[wenlei]

lgtm, thanks.

[hoy]

Fixing test failures by explictly turning off profi.

llvm/lib/Transforms/IPO/SampleProfile.cpp

ExternalInlineAdvisor = getReplayInlineAdvisor(
         M, *FAM, Ctx, /*OriginalAdvisor=*/nullptr,
         ReplayInlinerSettings{ProfileInlineReplayFile,
                               ProfileInlineReplayScope,
                               ProfileInlineReplayFallback,
                               {ProfileInlineReplayFormat}},
         /*EmitRemarks=*/false);
   }
 
-  // Apply tweaks if context-sensitive profile is available.
+  // Apply tweaks if context-sensitive or probe-based profile is available.
+  if (Reader->profileIsCSFlat() || Reader->profileIsCSNested() ||
+      Reader->profileIsProbeBased()) {
+    if (!UseIterativeBFIInference.getNumOccurrences())
+      UseIterativeBFIInference = true;
+    if (!SampleProfileUseProfi.getNumOccurrences())
+      SampleProfileUseProfi = true;
+    if (!EnableExtTspBlockPlacement.getNumOccurrences())
+      EnableExtTspBlockPlacement = true;
+  }
+
   if (Reader->profileIsCSFlat() || Reader->profileIsCSNested()) {
     ProfileIsCSFlat = Reader->profileIsCSFlat();
     // 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;
 
-    // Enable iterative-BFI by default for CSSPGO.
-    if (!UseIterativeBFIInference.getNumOccurrences())
-      UseIterativeBFIInference = true;
-    // Enable Profi by default for CSSPGO.
-    if (!SampleProfileUseProfi.getNumOccurrences())
-      SampleProfileUseProfi = true;
-
-    // Enable EXT-TSP block layout for CSSPGO.
-    if (!EnableExtTspBlockPlacement.getNumOccurrences())
-      EnableExtTspBlockPlacement = true;
-
     if (FunctionSamples::ProfileIsCSFlat) {
       // 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);
     if (!ProbeManager->moduleIsProbed(M)) {
       const char *Msg =
           "Pseudo-probe-based profile requires SampleProfileProbePass";
       Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg,
                                                DS_Warning));
       return false;
     }
   }
 
   return true;
 }

llvm/test/Transforms/SampleProfile/function_metadata.ll

 ; RUN: opt < %s -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/function_metadata.prof -S | FileCheck %s
 ; RUN: opt < %s -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/function_metadata.compact.afdo -S | FileCheck %s
-; RUN: opt < %s -passes='pseudo-probe,thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/pseudo-probe-func-metadata.prof -S | FileCheck %s
+; RUN: opt < %s -passes='pseudo-probe,thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/pseudo-probe-func-metadata.prof -sample-profile-use-profi=0 -S | FileCheck %s
 
 ;; Validate that with replay in effect, we import call sites even if they are below the threshold
 ;; Baseline import decisions
 ; RUN: opt < %s -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-summary-hot-count=2000 -profile-file=%S/Inputs/function_metadata.prof -S | FileCheck -check-prefix=THRESHOLD %s
 ;; With replay decisions
 ; RUN: opt < %s -passes='thinlto-pre-link<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-summary-hot-count=2000 -profile-file=%S/Inputs/function_metadata.prof -S -sample-profile-inline-replay-scope=Function -sample-profile-inline-replay=%S/Inputs/function_metadata_replay.txt | FileCheck -check-prefix=THRESHOLD-REPLAY %s
 
 

llvm/test/Transforms/SampleProfile/profile-correlation-irreducible-loops.ll

-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-correlation-irreducible-loops.prof | opt -passes='print<block-freq>' -disable-output  -use-iterative-bfi-inference 2>&1 | FileCheck %s
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-correlation-irreducible-loops.prof -S | FileCheck %s --check-prefix=CHECK2
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-correlation-irreducible-loops.prof -sample-profile-use-profi=0 | opt -passes='print<block-freq>' -disable-output  -use-iterative-bfi-inference 2>&1 | FileCheck %s
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-correlation-irreducible-loops.prof -sample-profile-use-profi=0 -S | FileCheck %s --check-prefix=CHECK2
 ; RUN: opt < %s -passes='print<block-freq>' -use-iterative-bfi-inference -disable-output 2>&1 | FileCheck %s --check-prefix=CHECK3
 
 ; The C++ code for this test case is from c-parse.c in 403.gcc (SPEC2006)
 ; The problem with BFI for the test is solved by applying iterative inference.
 ; The corresponding CFG graph is shown below, with intended counts for every
 ; basic block. The hot loop, b3->b4->b2, is not getting proper (large) counts
 ; unless the -use-iterative-bfi-inference option is specified.
 ;

llvm/test/Transforms/SampleProfile/pseudo-probe-profile-metadata.ll

-; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-metadata.prof -S | FileCheck %s
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-metadata.prof -sample-profile-use-profi=0 -S | FileCheck %s
 
 ; The test verifies the presence of prof metadata for BranchInst, SwitchInst,
 ; and IndirectBrInst
 
 @yydebug = dso_local global i32 0, align 4
 
 define dso_local i32 @foo() #0 {
 entry:

llvm/test/Transforms/SampleProfile/pseudo-probe-profile.ll

-; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile.prof -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml -S | FileCheck %s
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile.prof -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml -sample-profile-use-profi=0 -S | FileCheck %s
 ; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml
 
 define dso_local i32 @foo(i32 %x, void (i32)* %f) #0 !dbg !4 {
 entry:
   %retval = alloca i32, align 4
   %x.addr = alloca i32, align 4
   store i32 %x, i32* %x.addr, align 4
   %0 = load i32, i32* %x.addr, align 4