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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added reviewers: wmi, rajeshwarv.
Herald added subscribers: ormris, wenlei, arphaman, steven_wu, hiraditya, inglorion.
tejohnson requested review of this revision.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110296

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


Index: llvm/test/ThinLTO/X86/distributed_indexes.ll
===================================================================
--- llvm/test/ThinLTO/X86/distributed_indexes.ll
+++ 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 @@
   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
+}
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===================================================================
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4148,7 +4148,14 @@
   // 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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110296.374398.patch
Type: text/x-patch
Size: 2695 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210922/95f5dce2/attachment.bin>