[llvm] 2c1defe - [ThinLTO] Don't emit original GUID for locals to distributed indexes

[Teresa Johnson via llvm-commits]

Author: Teresa Johnson
Date: 2021-09-23T17:35:47-07:00
New Revision: 2c1defeee40cf643ea6f0fa5e01164c9a4c48c30

URL: https://github.com/llvm/llvm-project/commit/2c1defeee40cf643ea6f0fa5e01164c9a4c48c30
DIFF: https://github.com/llvm/llvm-project/commit/2c1defeee40cf643ea6f0fa5e01164c9a4c48c30.diff

LOG: [ThinLTO] Don't emit original GUID for locals to distributed indexes

In ThinLTO for locals we normally compute the GUID from the name after
prepending the source path to get a unique global id. SamplePGO indirect
call profiles contain the target GUID without this uniquification,
however (unless compiling with -funique-internal-linkage-names).
Therefore, the index contains the original GUID of the local symbols
(without module path prepended to uniquify), in order to correctly
handle the call edges added for these indirect call profile targets
with SamplePGO.

We were emitting these to the combined index when writing it out as
bitcode, which is unnecessary and causes overhead when writing out the
indexes for distributed backends. The only use of the original GUID name
is in the thin link. Suppress it in that case. This reduced the thin
link time for a large distributed build by about 7%, and the aggregate
size of the serialized indexes by over 2%.

Continue to print it when writing out the full index, since that is just
used for debugging and testing.

Update a distributed thinlto index test to contain a local and ensure
that we don't get a COMBINED_ORIGINAL_NAME record.

Differential Revision: https://reviews.llvm.org/D110296

Added: 
    

Modified: 
    llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
    llvm/test/ThinLTO/X86/distributed_indexes.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 24b6e165ae7b4..402840eab6a28 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4148,7 +4148,14 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
   // For local linkage, we also emit the original name separately
   // immediately after the record.
   auto MaybeEmitOriginalName = [&](GlobalValueSummary &S) {
-    if (!GlobalValue::isLocalLinkage(S.linkage()))
+    // We don't need to emit the original name if we are writing the index for
+    // distributed backends (in which case ModuleToSummariesForIndex is
+    // non-null). The original name is only needed during the thin link, since
+    // for SamplePGO the indirect call targets for local functions have
+    // have the original name annotated in profile.
+    // Continue to emit it when writing out the entire combined index, which is
+    // used in testing the thin link via llvm-lto.
+    if (ModuleToSummariesForIndex || !GlobalValue::isLocalLinkage(S.linkage()))
       return;
     NameVals.push_back(S.getOriginalName());
     Stream.EmitRecord(bitc::FS_COMBINED_ORIGINAL_NAME, NameVals);

diff  --git a/llvm/test/ThinLTO/X86/distributed_indexes.ll b/llvm/test/ThinLTO/X86/distributed_indexes.ll
index 65f14a2cb5b86..245ece4f97368 100644
--- a/llvm/test/ThinLTO/X86/distributed_indexes.ll
+++ b/llvm/test/ThinLTO/X86/distributed_indexes.ll
@@ -20,10 +20,12 @@
 ; BACKEND1-DAG: <VALUE_GUID op0={{.*}} op1=-5300342847281564238
 ; BACKEND1-DAG: <VALUE_GUID op0={{.*}} op1=-3706093650706652785
 ; BACKEND1-DAG: <VALUE_GUID op0={{.*}} op1=-1039159065113703048
-; BACKEND1-DAG: <COMBINED {{.*}} op1=0
-; BACKEND1-DAG: <COMBINED {{.*}} op1=1
-; BACKEND1-DAG: <COMBINED_ALIAS {{.*}} op1=1
-; BACKEND1-NEXT: <BLOCK_COUNT op0=3/>
+; BACKEND1-DAG: <VALUE_GUID op0={{.*}} op1=-126160295109682753
+; BACKEND1-NEXT: <COMBINED {{.*}} op1=0
+; BACKEND1-NEXT: <COMBINED {{.*}} op1=0
+; BACKEND1-NEXT: <COMBINED {{.*}} op1=1
+; BACKEND1-NEXT: <COMBINED_ALIAS {{.*}} op1=1
+; BACKEND1-NEXT: <BLOCK_COUNT op0=4/>
 ; BACKEND1-NEXT: </GLOBALVAL_SUMMARY_BLOCK
 
 ; The backend index for Input/distributed_indexes.ll contains summaries from
@@ -39,7 +41,7 @@
 ; BACKEND2-NEXT: <COMBINED
 ; BACKEND2-NEXT: <COMBINED
 ; BACKEND2-NEXT: <COMBINED_ALIAS
-; BACKEND2-NEXT: <BLOCK_COUNT op0=3/>
+; BACKEND2-NEXT: <BLOCK_COUNT op0=4/>
 ; BACKEND2-NEXT: </GLOBALVAL_SUMMARY_BLOCK
 
 ; Make sure that when the alias is imported as a copy of the aliasee, but the
@@ -59,3 +61,9 @@ entry:
   call void (...) @analias()
   ret void
 }
+
+; Ensure we don't get a COMBINED_ORIGINAL_NAME record in the distributed index.
+; The BACKEND1-NEXT checks would fail if we did.
+define internal void @x() {
+  ret void
+}