[CSSPGO][PriorityInliner] Do not use block weight to drive callsite inlining.

[hoy]

Summary

The priority-based inliner currenlty uses block count combined with callee entry count to drive callsite inlining. This doesn't work well with LTO where postlink inlining is driven by prelink-annotated block count which could be based on the merge of all context profiles. I'm fixing it by using callee profile entry count only which should be context-sensitive.

I'm seeing 0.2% perf improvment for one of our internal large benchmarks with probe-based non-CS profile.

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1309

When we don't have callee samples, should we fall back to call site block counts?

In reality we would also need to tolerate some source change, i.e. the call site didn't exist in pass1 build.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1309

If the callsite doesn't exist in pass1, the caller profile will probably be discarded due to checksum mismatch. So here when callee sample is missing, it's likely that the callsite is cold in this particular context. Using block count might end up treating it as hot.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1309

Interesting point on tolerating source changes. That would require some changes to probe numbering. So far we number callsite probes sequentially which is easily broken with a new callsite introduced. We might somehow need to keep this numbering stable across builds.

[wenlei]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1309

If the callsite doesn't exist in pass1, the caller profile will probably be discarded due to checksum mismatch.

Is it? If that's the behavior today, it defeats the purpose of CFG based profile - we should be able to match as long as CFG does not change. we can have a change that adds a call site without changing CFG.

[hoy]

llvm/lib/Transforms/IPO/SampleProfile.cpp

1309

Yes, callsites are currently considered when computing the function checksum:

FunctionHash = (uint64_t)CallProbeIds.size() << 48 |
(uint64_t)Indexes.size() << 32 | JC.getCRC();

We do this to prevent callsite samples mismatch. E.g, indirect call target samples can be applied to an irrelevant callsite when the number of callsite changes.

[wenlei]

lgtm, thanks.

llvm/lib/Transforms/IPO/SampleProfile.cpp

1309

This is something we need to address eventually. The design of CSSPGO is made to tolerate changes not altering CFG, but looks like the current implementation does not satisfy that yet.. For now this change looks fine.

llvm/lib/Transforms/IPO/SampleProfile.cpp

bool SampleProfileLoader::getInlineCandidate(InlineCandidate *NewCandidate,
   // if Samples are not present.
   if (!CalleeSamples && !getExternalInlineAdvisorShouldInline(*CB))
     return false;
 
   float Factor = 1.0;
   if (Optional<PseudoProbe> Probe = extractProbe(*CB))
     Factor = Probe->Factor;
 
-  uint64_t CallsiteCount = 0;
-  ErrorOr<uint64_t> Weight = getBlockWeight(CB->getParent());
-  if (Weight)
-    CallsiteCount = Weight.get();
-  if (CalleeSamples)
-    CallsiteCount = std::max(
-        CallsiteCount, uint64_t(CalleeSamples->getEntrySamples() * Factor));
-
+  uint64_t CallsiteCount =
+      CalleeSamples ? CalleeSamples->getEntrySamples() * Factor : 0;
   *NewCandidate = {CB, CalleeSamples, CallsiteCount, Factor};
   return true;
 }
 
 Optional<InlineCost>
 SampleProfileLoader::getExternalInlineAdvisorCost(CallBase &CB) {
   std::unique_ptr<InlineAdvice> Advice = nullptr;
   if (ExternalInlineAdvisor) {

llvm/test/Transforms/SampleProfile/Inputs/profile-context-order-scc.prof

-[main:3 @ _Z5funcAi:1 @ _Z8funcLeafi]:1467299:11
+[main:3 @ _Z5funcAi:1 @ _Z8funcLeafi]:1467299:287864
  0: 6
  1: 6
  3: 287884
  15: 23
-[main:3.1 @ _Z5funcBi:1 @ _Z8funcLeafi]:500853:20
+[main:3.1 @ _Z5funcBi:1 @ _Z8funcLeafi]:500853:287864
  0: 15
  1: 15
  3: 74946
  10: 23324
  15: 11
 [main]:154:0
  2: 12
  3: 18 _Z5funcAi:11
  3.1: 18 _Z5funcBi:19
 [external:12 @ main]:154:12
  2: 12
  3: 10 _Z5funcAi:7
  3.1: 10 _Z5funcBi:11
 [main:3.1 @ _Z5funcBi]:120:19
  0: 19
- 1: 19 _Z8funcLeafi:20
+ 1: 287864 _Z8funcLeafi:287864
  3: 12
 [externalA:17 @ _Z5funcBi]:120:3
  0: 3
  1: 3
 [external:10 @ _Z5funcBi]:120:10
  0: 10
  1: 10
 [main:3 @ _Z5funcAi]:99:11
  0: 10
- 1: 10 _Z8funcLeafi:11
+ 1: 287864 _Z8funcLeafi:287864
  2: 287864 _Z3fibi:315608
  3: 24
 [main:3 @ _Z5funcAi:2 @ _Z3fibi]:287864:315608
  0: 362839
  1: 6
  3: 287884
 [main:3 @ _Z5funcAi:1 @ _Z8funcLeafi:1 @ _Z5funcBi]:1467299:6
  0: 6
  1: 6
- 3: 287884
- 15: 23
+ 3: 6
 

llvm/test/Transforms/SampleProfile/Inputs/profile-context-order.prof

-[main:3 @ _Z5funcAi:1 @ _Z8funcLeafi]:1467299:11
+[main:3 @ _Z5funcAi:1 @ _Z8funcLeafi]:1467299:287864
  0: 6
  1: 6
  3: 287884
  15: 23
 [main:3.1 @ _Z5funcBi:1 @ _Z8funcLeafi]:500853:20
  0: 15
  1: 15
  3: 74946

 [externalA:17 @ _Z5funcBi]:120:3
  0: 3
  1: 3
 [external:10 @ _Z5funcBi]:120:10
  0: 10
  1: 10
 [main:3 @ _Z5funcAi]:99:11
  0: 10
- 1: 10 _Z8funcLeafi:11
+ 1: 287864 _Z8funcLeafi:287864
  2: 287864 _Z3fibi:315608
  3: 24
 [main:3 @ _Z5funcAi:2 @ _Z3fibi]:287864:315608
  0: 362839
  1: 6
- 3: 287884
+ 3: 287884
 

llvm/test/Transforms/SampleProfile/csspgo-inline-icall.ll

 !6 = !DILocation(line: 6, scope: !3)
 !7 = !DILocation(line: 7, scope: !3)
 !8 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 29, unit: !0)
 !9 = distinct !DISubprogram(name: "bar", linkageName: "_Z3barv", scope: !1, file: !1, line: 32, unit: !0)
 !10 = distinct !DISubprogram(name: "baz", linkageName: "_Z3bazv", scope: !1, file: !1, line: 24, unit: !0)
 !11 = distinct !DISubprogram(name: "zoo", linkageName: "_Z3zoov", scope: !1, file: !1, line: 24, unit: !0)
 
 
-; ICP-ALL: remark: test.cc:5:0: '_Z3bazv' inlined into 'test'
-; ICP-ALL-NEXT: remark: test.cc:4:0: '_Z3foov' inlined into 'test'
+; ICP-ALL:      remark: test.cc:4:0: '_Z3foov' inlined into 'test'
 ; ICP-ALL-NEXT: remark: test.cc:4:0: '_Z3barv' inlined into 'test'
+; ICP-ALL-NEXT: remark: test.cc:5:0: '_Z3bazv' inlined into 'test'
 ; ICP-ALL-NOT: remark
 
 ; ICP-HOT: remark: test.cc:4:0: '_Z3foov' inlined into 'test'
 ; ICP-HOT-NOT: remark

llvm/test/Transforms/SampleProfile/csspgo-inline.ll

 ; RUN: llvm-profdata merge --sample --text --gen-cs-nested-profile %S/Inputs/profile-context-tracker.prof -o %t.prof
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%t.prof -sample-profile-inline-size -sample-profile-prioritized-inline=0 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-BASE
 
 ; With new FDO early inliner, callee entry count is used to drive inlining instead of callee total samples, so we get less inlining for given profile
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-NEW
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%t.prof -sample-profile-prioritized-inline -sample-profile-inline-size -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-NEW
 ;
 ; With new FDO early inliner, callee entry count is used to drive inlining instead of callee total samples, tuning hot cutoff can get us the same inlining
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-summary-cutoff-hot=999900 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-BASE
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-summary-cutoff-hot=999990 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-BASE
 ;
 ; With new FDO early inliner, callee entry count is used to drive inlining instead of callee total samples, tuning cold sample profile inline threshold can get us the same inlining
 ; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -sample-profile-cold-inline-threshold=200 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-BASE
 ;
 ; With new FDO early inliner and tuned cutoff, we can control inlining through size growth tuning knob.
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-summary-cutoff-hot=999900 -sample-profile-inline-limit-min=0 -sample-profile-inline-growth-limit=1 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --allow-empty --check-prefix=INLINE-NEW-LIMIT1
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-summary-cutoff-hot=999900 -sample-profile-inline-limit-min=10 -sample-profile-inline-growth-limit=1 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-NEW-LIMIT2
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-summary-cutoff-hot=999990 -sample-profile-inline-limit-min=0 -sample-profile-inline-growth-limit=1 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --allow-empty --check-prefix=INLINE-NEW-LIMIT1
+; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/profile-context-tracker.prof -sample-profile-inline-size -profile-summary-cutoff-hot=999990 -sample-profile-inline-limit-min=10 -sample-profile-inline-growth-limit=1 -profile-sample-accurate -S -pass-remarks=inline -o /dev/null 2>&1 | FileCheck %s --check-prefix=INLINE-NEW-LIMIT2
 
 
 ; INLINE-BASE: remark: merged.cpp:14:10: '_Z5funcAi' inlined into 'main' to match profiling context with (cost={{[0-9]+}}, threshold={{[0-9]+}}) at callsite main:3:10
 ; INLINE-BASE: remark: merged.cpp:27:11: '_Z8funcLeafi' inlined into 'main' to match profiling context with (cost={{[0-9]+}}, threshold={{[0-9]+}}) at callsite _Z5funcAi:1:11 @ main:3:10
 ; INLINE-BASE: remark: merged.cpp:33:11: '_Z8funcLeafi' inlined into '_Z5funcBi' to match profiling context with (cost={{[0-9]+}}, threshold={{[0-9]+}}) at callsite _Z5funcBi:1:11
 
 ; INLINE-NEW: remark: merged.cpp:14:10: '_Z5funcAi' inlined into 'main' to match profiling context with (cost={{[0-9]+}}, threshold={{[0-9]+}}) at callsite main:3:10
 ; INLINE-NEW-NOT: remark
 
 ; INLINE-NEW-LIMIT1-NOT: remark
 
 ; INLINE-NEW-LIMIT2: remark: merged.cpp:33:11: '_Z8funcLeafi' inlined into '_Z5funcBi' to match profiling context with (cost={{[0-9]+}}, threshold={{[0-9]+}}) at callsite _Z5funcBi:1:11
+; INLINE-NEW-LIMIT2: remark: merged.cpp:27:11: '_Z8funcLeafi' inlined into '_Z5funcAi' to match profiling context with (cost={{[0-9]+}}, threshold={{[0-9]+}}) at callsite _Z5funcAi:1:11;
 ; INLINE-NEW-LIMIT2-NOT: remark
 
 @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