[PATCH] D33790: [ThinLTO] Assign ValueId only once per GUID when writing combined summary

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
Herald added subscribers: eraman, inglorion, Prazek, mehdi_amini.

Minor efficiency improvement when writing combined summaries (e.g. for
distributed back ends). For symbols that had multiple summaries (e.g.
linkonce or weak), we were assigning a value id multiple times. Add a
new helper to walk only the unique GUID in the index, instead of all
summaries.


https://reviews.llvm.org/D33790

Files:
  lib/Bitcode/Writer/BitcodeWriter.cpp
  test/Bitcode/Inputs/thinlto-function-summary-callgraph-linkonce.ll
  test/Bitcode/thinlto-function-summary-callgraph-linkonce.ll


Index: test/Bitcode/thinlto-function-summary-callgraph-linkonce.ll
===================================================================
--- /dev/null
+++ test/Bitcode/thinlto-function-summary-callgraph-linkonce.ll
@@ -0,0 +1,36 @@
+; Test to check for correct value numbering when there are multiple copies of a
+; (linkonce) function.
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto-function-summary-callgraph-linkonce.ll -o %t2.o
+; RUN: llvm-lto -thinlto -o %t3 %t.o %t2.o
+; RUN: llvm-bcanalyzer -dump %t3.thinlto.bc | FileCheck %s --check-prefix=COMBINED
+
+; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
+; COMBINED-NEXT:    <VERSION
+; func:
+; COMBINED-NEXT:    <VALUE_GUID op0=1 op1=7289175272376759421/>
+; main:
+; COMBINED-NEXT:    <VALUE_GUID op0=2
+; Two summaries for func:
+; COMBINED-NEXT:    <COMBINED {{.*}} op0=1
+; COMBINED-NEXT:    <COMBINED {{.*}} op0=1
+; main should have an edge to func:
+; COMBINED-NEXT:    <COMBINED {{.*}} op5=1/>
+; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
+
+; ModuleID = 'thinlto-function-summary-callgraph-linkonce.ll'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define linkonce_odr void @func() #0 {
+entry:
+      ret void
+}
+
+; Function Attrs: nounwind uwtable
+define void @main() #0 {
+entry:
+    call void () @func()
+    ret void
+}
Index: test/Bitcode/Inputs/thinlto-function-summary-callgraph-linkonce.ll
===================================================================
--- /dev/null
+++ test/Bitcode/Inputs/thinlto-function-summary-callgraph-linkonce.ll
@@ -0,0 +1,9 @@
+; ModuleID = 'thinlto-function-summary-callgraph-linkonce2.ll'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: nounwind uwtable
+define linkonce_odr void @func() #0 {
+entry:
+    ret void
+}
Index: lib/Bitcode/Writer/BitcodeWriter.cpp
===================================================================
--- lib/Bitcode/Writer/BitcodeWriter.cpp
+++ lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -336,18 +336,33 @@
                          *ModuleToSummariesForIndex = nullptr)
       : BitcodeWriterBase(Stream), Index(Index),
         ModuleToSummariesForIndex(ModuleToSummariesForIndex) {
-    // Assign unique value ids to all summaries to be written, for use
-    // in writing out the call graph edges. Save the mapping from GUID
+    // Assign unique value ids to all unique GUID with summaries to be written,
+    // for use in writing out the call graph edges. Save the mapping from GUID
     // to the new global value id to use when writing those edges, which
     // are currently saved in the index in terms of GUID.
-    forEachSummary([&](GVInfo I) {
-      GUIDToValueIdMap[I.first] = ++GlobalValueId;
+    forEachGUID([&](GlobalValue::GUID GUID) {
+      GUIDToValueIdMap[GUID] = ++GlobalValueId;
     });
   }
 
   /// The below iterator returns the GUID and associated summary.
   typedef std::pair<GlobalValue::GUID, GlobalValueSummary *> GVInfo;
 
+  /// Calls the callback for each value GUID with a summary to be written to
+  /// bitcode. This hides the details of whether they are being pulled from the
+  /// entire index or just those in a provided ModuleToSummariesForIndex map.
+  void forEachGUID(std::function<void(GlobalValue::GUID)> Callback) {
+    if (ModuleToSummariesForIndex) {
+      for (auto &M : *ModuleToSummariesForIndex)
+        for (auto &Summary : M.second)
+          Callback(Summary.first);
+    } else {
+      for (auto &Summaries : Index)
+        if (Summaries.second.SummaryList.size())
+          Callback(Summaries.first);
+    }
+  }
+
   /// Calls the callback for each value GUID and summary to be written to
   /// bitcode. This hides the details of whether they are being pulled from the
   /// entire index or just those in a provided ModuleToSummariesForIndex map.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33790.101052.patch
Type: text/x-patch
Size: 3973 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170601/08c44b6f/attachment.bin>