[PATCH] D151249: [ThinLTO] Disable partial sample profile scaling by default

Tue May 23 13:11:28 PDT 2023

tejohnson created this revision.
tejohnson added reviewers: davidxl, huangjd.
Herald added subscribers: ormris, wenlei, steven_wu, hiraditya, inglorion.
Herald added a project: All.
tejohnson requested review of this revision.
Herald added a project: LLVM.

As pointed out in
https://discourse.llvm.org/t/undeterministic-thin-index-file/69985, the
block count added to distributed ThinLTO index files breaks incremental
builds on ThinLTO - if any linked file has a different number of BBs,
then the accumulated sum placed in the index files will change, causing
all ThinLTO backend compiles to be redone.

This was only used for partial sample profiles, and was therefore
removed for other cases (3adc6e03080c6d38a51f5c5b6744b7c0d9c7541b <https://reviews.llvm.org/rG3adc6e03080c6d38a51f5c5b6744b7c0d9c7541b>).

Subsequent testing did not show a performance effect of disabling this
feature even for partial sample profiles. Therefore, switch the default
to false. If this does not cause a noticeable performance degradation
after the default flip, we can remove this support completely.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151249

Files:
  llvm/lib/Analysis/ProfileSummaryInfo.cpp
  llvm/test/Bitcode/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll


Index: llvm/test/Bitcode/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll
===================================================================

--- llvm/test/Bitcode/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll
+++ llvm/test/Bitcode/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll
@@ -1,43 +1,50 @@
-; Test to check the callgraph in summary when there is PGO
-; RUN: opt -module-summary %s -o %t.o
-; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
-; RUN: opt -module-summary %p/Inputs/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll -o %t2.o
+;; Test to check the callgraph in summary when partial sample profile scaling
+;; enabled.
+; RUN: opt -module-summary %s -o %t.o -scale-partial-sample-profile-working-set-size
+; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s --check-prefix=PERMODULE
+; RUN: opt -module-summary %p/Inputs/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll -o %t2.o -scale-partial-sample-profile-working-set-size
 ; RUN: llvm-lto -thinlto -o %t3 %t.o %t2.o
 ; RUN: llvm-bcanalyzer -dump %t3.thinlto.bc | FileCheck %s --check-prefix=COMBINED
 
+;; Check that we don't get block count records when it isn't explicitly enabled.
+; RUN: opt -module-summary %s -o %t.o
+; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s --implicit-check-not=BLOCK_COUNT
+; RUN: opt -module-summary %p/Inputs/thinlto-function-summary-callgraph-partial-sample-profile-summary.ll -o %t2.o
+; RUN: llvm-lto -thinlto -o %t3 %t.o %t2.o
+; RUN: llvm-bcanalyzer -dump %t3.thinlto.bc | FileCheck %s --implicit-check-not=BLOCK_COUNT
 
-; CHECK: <SOURCE_FILENAME
+; PERMODULE: <SOURCE_FILENAME
 ; "hot_function"
-; CHECK-NEXT: <FUNCTION op0=0 op1=12
+; PERMODULE-NEXT: <FUNCTION op0=0 op1=12
 ; "hot1"
-; CHECK-NEXT: <FUNCTION op0=12 op1=4
+; PERMODULE-NEXT: <FUNCTION op0=12 op1=4
 ; "hot2"
-; CHECK-NEXT: <FUNCTION op0=16 op1=4
+; PERMODULE-NEXT: <FUNCTION op0=16 op1=4
 ; "hot3"
-; CHECK-NEXT: <FUNCTION op0=20 op1=4
+; PERMODULE-NEXT: <FUNCTION op0=20 op1=4
 ; "hot4"
-; CHECK-NEXT: <FUNCTION op0=24 op1=5
+; PERMODULE-NEXT: <FUNCTION op0=24 op1=5
 ; "cold"
-; CHECK-NEXT: <FUNCTION op0=29 op1=5
+; PERMODULE-NEXT: <FUNCTION op0=29 op1=5
 ; "none1"
-; CHECK-NEXT: <FUNCTION op0=34 op1=5
+; PERMODULE-NEXT: <FUNCTION op0=34 op1=5
 ; "none2"
-; CHECK-NEXT: <FUNCTION op0=39 op1=5
+; PERMODULE-NEXT: <FUNCTION op0=39 op1=5
 ; "none3"
-; CHECK-NEXT: <FUNCTION op0=44 op1=5
-; CHECK-NEXT: <FUNCTION op0=49 op1=5
+; PERMODULE-NEXT: <FUNCTION op0=44 op1=5
+; PERMODULE-NEXT: <FUNCTION op0=49 op1=5
 
-; CHECK-LABEL:       <GLOBALVAL_SUMMARY_BLOCK
-; CHECK-NEXT:    <VERSION
-; CHECK-NEXT:    <FLAGS
-; CHECK-NEXT:    <VALUE_GUID op0=27 op1=123/>
+; PERMODULE-LABEL:       <GLOBALVAL_SUMMARY_BLOCK
+; PERMODULE-NEXT:    <VERSION
+; PERMODULE-NEXT:    <FLAGS
+; PERMODULE-NEXT:    <VALUE_GUID op0=27 op1=123/>
 ; op4=none1 op6=hot1 op8=cold1 op10=none2 op12=hot2 op14=cold2 op16=none3 op18=hot3 op20=cold3 op22=123
-; CHECK-NEXT:    <PERMODULE_PROFILE {{.*}} op7=7 op8=0 op9=1 op10=3 op11=4 op12=1 op13=8 op14=0 op15=2 op16=3 op17=5 op18=1 op19=9 op20=0 op21=3 op22=3 op23=6 op24=1 op25=27 op26=4/>
-; CHECK-NEXT:    <BLOCK_COUNT op0=4/>
-; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
+; PERMODULE-NEXT:    <PERMODULE_PROFILE {{.*}} op7=7 op8=0 op9=1 op10=3 op11=4 op12=1 op13=8 op14=0 op15=2 op16=3 op17=5 op18=1 op19=9 op20=0 op21=3 op22=3 op23=6 op24=1 op25=27 op26=4/>
+; PERMODULE-NEXT:    <BLOCK_COUNT op0=4/>
+; PERMODULE-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
-; CHECK: <STRTAB_BLOCK
-; CHECK-NEXT: blob data = 'hot_functionhot1hot2hot3cold1cold2cold3none1none2none3{{.*}}'
+; PERMODULE: <STRTAB_BLOCK
+; PERMODULE-NEXT: blob data = 'hot_functionhot1hot2hot3cold1cold2cold3none1none2none3{{.*}}'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
Index: llvm/lib/Analysis/ProfileSummaryInfo.cpp
===================================================================
--- llvm/lib/Analysis/ProfileSummaryInfo.cpp
+++ llvm/lib/Analysis/ProfileSummaryInfo.cpp
@@ -37,8 +37,13 @@
     "partial-profile", cl::Hidden, cl::init(false),
     cl::desc("Specify the current profile is used as a partial profile."));
 
+// TODO: Remove this support completely after ensuring that disabling by
+// default has no unexpected effects. This causes the global number of basic
+// blocks to be recorded in the ThinLTO summary, which breaks caching in the
+// distributed ThinLTO case.
 cl::opt<bool> ScalePartialSampleProfileWorkingSetSize(
-    "scale-partial-sample-profile-working-set-size", cl::Hidden, cl::init(true),
+    "scale-partial-sample-profile-working-set-size", cl::Hidden,
+    cl::init(false),
     cl::desc(
         "If true, scale the working set size of the partial sample profile "
         "by the partial profile ratio to reflect the size of the program "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151249.524855.patch
Type: text/x-patch
Size: 4876 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230523/1bb052d3/attachment-0001.bin>
