[ThinLTO] Indirect call promotion fixes for promoted local functions

[tejohnson]

Summary

Fix a couple issues limiting the application of indirect call promotion
in ThinLTO mode:

• Invoke indirect call promotion before globalopt, since it may eliminate imported functions which appear unreferenced.
• Invoke indirect call promotion with InLTO=true so that the PGOFuncName metadata is used to get the name for locals which would have been renamed during promotion.

[davidxl]

lib/Transforms/IPO/PassManagerBuilder.cpp

412 ↗

(On Diff #69615)

Is there PGOInstr/Annotation pass before this?

[tejohnson]

lib/Transforms/IPO/PassManagerBuilder.cpp

412 ↗

(On Diff #69615)

In the compile step. PerformThinLTO is true when we are in the ThinLTO backends. See also further down where we guard the invocation of addPGOInstrPasses with !PerformThinLTO.

[Prazek]

SGTM, couldn't find anything unclear.

[davidxl]

lib/Transforms/IPO/PassManagerBuilder.cpp

412 ↗

(On Diff #69615)

Ok. Can you add a comment here documenting this: with thinLTO, there are two icall promotion pass. One is in the compile phase where PerformThinLTO is false to handle intra-module calls, the other one is here that does cross module ones?

[mehdi_amini]

LGTM as well.

lib/Transforms/IPO/PassManagerBuilder.cpp

376 ↗

(On Diff #69615)

Spurious empty line?

440 ↗

(On Diff #69615)

Do we want to do this in the compile phase?

[tejohnson]

lib/Transforms/IPO/PassManagerBuilder.cpp

376 ↗

(On Diff #69615)

Will remove

412 ↗

(On Diff #69615)

Will do.

440 ↗

(On Diff #69615)

Yes, we want to have one in the compile phase to catch intra-module indirect calls (that will subsequently be eligible for inlining during the compile phase).

llvm/trunk/lib/ProfileData/InstrProf.cpp

std::string getPGOFuncName(StringRef RawFuncName,
   return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName);
 }
 
 // Return the PGOFuncName. This function has some special handling when called
 // in LTO optimization. The following only applies when calling in LTO passes
 // (when \c InLTO is true): LTO's internalization privatizes many global linkage
 // symbols. This happens after value profile annotation, but those internal
 // linkage functions should not have a source prefix.
+// Additionally, for ThinLTO mode, exported internal functions are promoted
+// and renamed. We need to ensure that the original internal PGO name is
+// used when computing the GUID that is compared against the profiled GUIDs.
 // To differentiate compiler generated internal symbols from original ones,
 // PGOFuncName meta data are created and attached to the original internal
 // symbols in the value profile annotation step
 // (PGOUseFunc::annotateIndirectCallSites). If a symbol does not have the meta
 // data, its original linkage must be non-internal.
 std::string getPGOFuncName(const Function &F, bool InLTO, uint64_t Version) {
   if (!InLTO) {
     StringRef FileName = (StaticFuncFullModulePrefix

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

void PassManagerBuilder::populateModulePassManager(
   }
 
   // Add LibraryInfo if we have some.
   if (LibraryInfo)
     MPM.add(new TargetLibraryInfoWrapperPass(*LibraryInfo));
 
   addInitialAliasAnalysisPasses(MPM);
 
+  // For ThinLTO there are two passes of indirect call promotion. The
+  // first is during the compile phase when PerformThinLTO=false and
+  // intra-module indirect call targets are promoted. The second is during
+  // the ThinLTO backend when PerformThinLTO=true, when we promote imported
+  // inter-module indirect calls. For that we perform indirect call promotion
+  // earlier in the pass pipeline, here before globalopt. Otherwise imported
+  // available_externally functions look unreferenced and are removed.
+  if (PerformThinLTO)
+    MPM.add(createPGOIndirectCallPromotionLegacyPass(/*InLTO = */ true));
+
   if (!DisableUnitAtATime) {
     // Infer attributes about declarations if possible.
     MPM.add(createInferFunctionAttrsLegacyPass());
 
     addExtensionsToPM(EP_ModuleOptimizerEarly, MPM);
 
     MPM.add(createIPSCCPPass());          // IP SCCP
     MPM.add(createGlobalOptimizerPass()); // Optimize out global vars
     // Promote any localized global vars.
     MPM.add(createPromoteMemoryToRegisterPass());
 
     MPM.add(createDeadArgEliminationPass()); // Dead argument elimination
 
     addInstructionCombiningPass(MPM); // Clean up after IPCP & DAE
     addExtensionsToPM(EP_Peephole, MPM);
     MPM.add(createCFGSimplificationPass()); // Clean up after IPCP & DAE
   }
 
   if (!PerformThinLTO) {
     /// PGO instrumentation is added during the compile phase for ThinLTO, do
     /// not run it a second time
     addPGOInstrPasses(MPM);
-  }
-
     // Indirect call promotion that promotes intra-module targets only.
+    // For ThinLTO this is done earlier due to interactions with globalopt
+    // for imported functions.
     MPM.add(createPGOIndirectCallPromotionLegacyPass());
+  }
 
   if (EnableNonLTOGlobalsModRef)
     // We add a module alias analysis pass here. In part due to bugs in the
     // analysis infrastructure this "works" in that the analysis stays alive
     // for the entire SCC pass run below.
     MPM.add(createGlobalsAAWrapperPass());
 
   // Start of CallGraph SCC passes.

llvm/trunk/test/Transforms/PGOProfile/Inputs/thinlto_indirect_call_promotion.ll

 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
+source_filename = "thinlto_indirect_call_promotion.c"
+
 define void @a() {
 entry:
   ret void
 }
+
+define internal void @c() !PGOFuncName !1 {
+entry:
+  ret void
+}
+
+!1 = !{!"thinlto_indirect_call_promotion.c:c"}

llvm/trunk/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: opt -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; RUN: opt -function-import -summary-file %t3.thinlto.bc %t.bc -o %t4.bc -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS
-; IMPORTS: Import a
+; IMPORTS-DAG: Import a
+; IMPORTS-DAG: Import c
 
-; RUN: opt %t4.bc -pgo-icall-prom -S -icp-count-threshold=1 | FileCheck %s --check-prefix=ICALL-PROM
-; RUN: opt %t4.bc -pgo-icall-prom -S -pass-remarks=pgo-icall-prom -icp-count-threshold=1 2>&1 | FileCheck %s --check-prefix=PASS-REMARK
+; RUN: opt %t4.bc -icp-lto -pgo-icall-prom -S -icp-count-threshold=1 | FileCheck %s --check-prefix=ICALL-PROM
+; RUN: opt %t4.bc -icp-lto -pgo-icall-prom -S -pass-remarks=pgo-icall-prom -icp-count-threshold=1 2>&1 | FileCheck %s --check-prefix=PASS-REMARK
 ; PASS-REMARK: Promote indirect call to a with count 1 out of 1
+; PASS-REMARK: Promote indirect call to c.llvm.0 with count 1 out of 1
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @foo = external local_unnamed_addr global void ()*, align 8
+@bar = external local_unnamed_addr global void ()*, align 8
 
 define i32 @main() local_unnamed_addr {
 entry:
   %0 = load void ()*, void ()** @foo, align 8
 ; ICALL-PROM:   br i1 %{{[0-9]+}}, label %if.true.direct_targ, label %if.false.orig_indirect, !prof [[BRANCH_WEIGHT:![0-9]+]]
   tail call void %0(), !prof !1
+  %1 = load void ()*, void ()** @bar, align 8
+; ICALL-PROM:   br i1 %{{[0-9]+}}, label %if.true.direct_targ1, label %if.false.orig_indirect2, !prof [[BRANCH_WEIGHT:![0-9]+]]
+  tail call void %1(), !prof !2
   ret i32 0
 }
 
 !1 = !{!"VP", i32 0, i64 1, i64 -6289574019528802036, i64 1}
+!2 = !{!"VP", i32 0, i64 1, i64 591260329866125152, i64 1}
 
 ; Should not have a VP annotation on new indirect call (check before and after
 ; branch_weights annotation).
 ; ICALL-PROM-NOT: !"VP"
 ; ICALL-PROM: [[BRANCH_WEIGHT]] = !{!"branch_weights", i32 1, i32 0}
 ; ICALL-PROM-NOT: !"VP"