[ThinLTO] Print names in function import debug messages when available

[tejohnson]

Summary

Rather than just print the GUID, when it is available in the index,
print the global name as well in the function import thin link debug
messages. Names will be available when the combined index is being
built by the same process, e.g. a linker or "llvm-lto2 run".

[tejohnson]

Updated description to clarify that this is for thin link debug messages, and the names are also available when being done via the linker. Only thin links invoked by e.g. llvm-lto on a serialized combined index will not have the names.

[davidxl]

Perhaps define a little helper function:

#ifndef NDEBUG
raw_ostream& operator << (raw_ostream&, ValueInfo&VI) {

 if (!VI.empty()) {
      ....
 }
return dbgs();

}
#endif

then this call can be embedded inside dbgs() << getGUID(VI) << VI << "\n";

[tejohnson]

In D48612#1144207, @davidxl wrote:

Perhaps define a little helper function:

#ifndef NDEBUG
raw_ostream& operator << (raw_ostream&, ValueInfo&VI) {

 if (!VI.empty()) {
      ....
 }
return dbgs();

}
#endif

then this call can be embedded inside dbgs() << getGUID(VI) << VI << "\n";

Good idea. I made it a bit more general, so that it prints both the GUID and Name (when available), which allows it to simplify the LLVM_DEBUG invocation further.

[tejohnson]

Make a ValueInfo printing helper.

[davidxl]

lgtm

lib/Transforms/IPO/FunctionImport.cpp

static void computeImportForReferencedGlobals(
     StringMap<FunctionImporter::ExportSetTy> *ExportLists) {
   for (auto &VI : Summary.refs()) {
     if (DefinedGVSummaries.count(VI.getGUID())) {
       LLVM_DEBUG(
           dbgs() << "Ref ignored! Target already in destination module.\n");
       continue;
     }
 
-    LLVM_DEBUG(dbgs() << " ref -> " << VI.getGUID() << "\n");
+    LLVM_DEBUG(dbgs() << " ref -> " << VI.getGUID());
+    if (!VI.name().empty())
+      LLVM_DEBUG(dbgs() << " (" << VI.name() << ")");
+    LLVM_DEBUG(dbgs() << "\n");
 
     for (auto &RefSummary : VI.getSummaryList())
       if (RefSummary->getSummaryKind() == GlobalValueSummary::GlobalVarKind &&
           // Don't try to import regular LTO summaries added to dummy module.
           !RefSummary->modulePath().empty() &&
           !GlobalValue::isInterposableLinkage(RefSummary->linkage()) &&
           RefSummary->refs().empty()) {
         ImportList[RefSummary->modulePath()][VI.getGUID()] = 1;
static void computeImportForFunction(
     SmallVectorImpl<EdgeInfo> &Worklist,
     FunctionImporter::ImportMapTy &ImportList,
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
   computeImportForReferencedGlobals(Summary, DefinedGVSummaries, ImportList,
                                     ExportLists);
   static int ImportCount = 0;
   for (auto &Edge : Summary.calls()) {
     ValueInfo VI = Edge.first;
-    LLVM_DEBUG(dbgs() << " edge -> " << VI.getGUID()
-                      << " Threshold:" << Threshold << "\n");
+    LLVM_DEBUG(dbgs() << " edge -> " << VI.getGUID());
+    if (!VI.name().empty())
+      LLVM_DEBUG(dbgs() << " (" << VI.name() << ")");
+    LLVM_DEBUG(dbgs() << " Threshold:" << Threshold << "\n");
 
     if (ImportCutoff >= 0 && ImportCount >= ImportCutoff) {
       LLVM_DEBUG(dbgs() << "ignored! import-cutoff value of " << ImportCutoff
                         << " reached.\n");
       continue;
     }
 
     VI = updateValueInfoForIndirectCalls(Index, VI);
static void ComputeImportForModule(
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
   // Worklist contains the list of function imported in this module, for which
   // we will analyse the callees and may import further down the callgraph.
   SmallVector<EdgeInfo, 128> Worklist;
 
   // Populate the worklist with the import for the functions in the current
   // module
   for (auto &GVSummary : DefinedGVSummaries) {
+#ifndef NDEBUG
+    // FIXME: Change the GVSummaryMapTy to hold ValueInfo instead of GUID
+    // so this map look up (and possibly others) can be avoided.
+    auto VI = Index.getValueInfo(GVSummary.first);
+#endif
     if (!Index.isGlobalValueLive(GVSummary.second)) {
-      LLVM_DEBUG(dbgs() << "Ignores Dead GUID: " << GVSummary.first << "\n");
+      LLVM_DEBUG(dbgs() << "Ignores Dead GUID: " << GVSummary.first);
+#ifndef NDEBUG
+      if (!VI.name().empty())
+        LLVM_DEBUG(dbgs() << " (" << VI.name() << ")");
+#endif
+      LLVM_DEBUG(dbgs() << "\n");
       continue;
     }
     auto *FuncSummary =
         dyn_cast<FunctionSummary>(GVSummary.second->getBaseObject());
     if (!FuncSummary)
       // Skip import for global variables
       continue;
-    LLVM_DEBUG(dbgs() << "Initialize import for " << GVSummary.first << "\n");
+    LLVM_DEBUG(dbgs() << "Initialize import for " << GVSummary.first);
+#ifndef NDEBUG
+    if (!VI.name().empty())
+      LLVM_DEBUG(dbgs() << " (" << VI.name() << ")");
+#endif
+    LLVM_DEBUG(dbgs() << "\n");
     computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
                              DefinedGVSummaries, Worklist, ImportList,
                              ExportLists);
   }
 
   // Process the newly imported functions and add callees to the worklist.
   while (!Worklist.empty()) {
     auto FuncInfo = Worklist.pop_back_val();
for (auto GUID : GUIDPreservedSymbols) {
     ValueInfo VI = Index.getValueInfo(GUID);
     if (!VI)
       continue;
     for (auto &S : VI.getSummaryList())
       S->setLive(true);
   }
 
   // Add values flagged in the index as live roots to the worklist.
-  for (const auto &Entry : Index)
+  for (const auto &Entry : Index) {
+    auto VI = Index.getValueInfo(Entry);
     for (auto &S : Entry.second.SummaryList)
       if (S->isLive()) {
-        LLVM_DEBUG(dbgs() << "Live root: " << Entry.first << "\n");
-        Worklist.push_back(ValueInfo(/*IsAnalysis=*/false, &Entry));
+        LLVM_DEBUG(dbgs() << "Live root: " << Entry.first);
+        if (!VI.name().empty())
+          LLVM_DEBUG(dbgs() << " (" << VI.name() << ")");
+        LLVM_DEBUG(dbgs() << "\n");
+        Worklist.push_back(VI);
         ++LiveSymbols;
         break;
       }
+  }
 
   // Make value live and add it to the worklist if it was not live before.
   auto visit = [&](ValueInfo VI) {
     // FIXME: If we knew which edges were created for indirect call profiles,
     // we could skip them here. Any that are live should be reached via
     // other edges, e.g. reference edges. Otherwise, using a profile collected
     // on a slightly different binary might provoke preserving, importing
     // and ultimately promoting calls to functions not linked into this

test/ThinLTO/X86/Inputs/funcimport-debug.ll

+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+
+define void @foo() #0 {
+entry:
+  ret void
+}
+
+@baz = constant i32 10, align 4

test/ThinLTO/X86/deadstrip.ll

+; Require asserts for -debug-only
+; REQUIRES: asserts
+
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/deadstrip.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
 
 ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t1.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t1.bc - -o - | llvm-dis -o - | FileCheck %s
 ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t2.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK2
 
 ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=run %t1.bc %t2.bc
 ; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s --check-prefix=CHECK-NM
 
 ; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.out -save-temps \
 ; RUN:   -r %t1.bc,_main,plx \
 ; RUN:   -r %t1.bc,_bar,pl \
 ; RUN:   -r %t1.bc,_dead_func,pl \
 ; RUN:   -r %t1.bc,_baz,l \
 ; RUN:   -r %t1.bc,_boo,l \
 ; RUN:   -r %t1.bc,_live_available_externally_func,l \
 ; RUN:   -r %t2.bc,_baz,pl \
 ; RUN:   -r %t2.bc,_boo,pl \
 ; RUN:   -r %t2.bc,_dead_func,l \
-; RUN:   -r %t2.bc,_another_dead_func,pl
+; RUN:   -r %t2.bc,_another_dead_func,pl \
+; RUN:   -thinlto-threads=1 \
+; RUN:   -debug-only=function-import 2>&1 | FileCheck %s --check-prefix=DEBUG
 ; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=LTO2
 ; RUN: llvm-dis < %t.out.2.3.import.bc | FileCheck %s --check-prefix=LTO2-CHECK2
 ; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM
 
 ; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=COMBINED
 ; Live, NotEligibleForImport, dso_local, Internal
 ; COMBINED-DAG: <COMBINED {{.*}} op2=119
 ; Live, dso_local, Internal

 ; The final binary should not contain any of the dead functions,
 ; only main is expected because bar is expected to be inlined and stripped out.
 ; CHECK-NM-NOT: bar
 ; CHECK-NM-NOT: dead
 ; CHECK-NM: T _main
 ; CHECK-NM-NOT: bar
 ; CHECK-NM-NOT: dead
 
+; DEBUG: Live root: 2412314959268824392 (llvm.global_ctors)
+; DEBUG: Live root: 15822663052811949562 (main)
+; DEBUG: Ignores Dead GUID: 7342339837106705152 (dead_func)
+; DEBUG: Ignores Dead GUID: 7546896869197086323 (baz)
+; DEBUG: Initialize import for 15611644523426561710 (boo)
+; DEBUG: Ignores Dead GUID: 2384416018110111308 (another_dead_func)
+
 ; Next test the case where Inputs/deadstrip.ll does not get a module index,
 ; which will cause it to be handled by regular LTO in the new LTO API.
 ; In that case there are uses of @dead_func in the regular LTO partition
 ; and it shouldn't be internalized.
 ; RUN: opt %p/Inputs/deadstrip.ll -o %t3.bc
 ; RUN: llvm-lto2 run %t1.bc %t3.bc -o %t4.out -save-temps \
 ; RUN:   -r %t1.bc,_main,plx \
 ; RUN:   -r %t1.bc,_bar,pl \

test/ThinLTO/X86/funcimport-debug.ll

+; Test of function importing debug messages
+
+; Require asserts for -debug-only
+; REQUIRES: asserts
+
+; Do setup work for all below tests: generate bitcode and combined index
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %p/Inputs/funcimport-debug.ll -o %t2.bc
+
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:     -r=%t1.bc,_main,plx \
+; RUN:     -r=%t1.bc,_foo,l \
+; RUN:     -r=%t1.bc,_baz,l \
+; RUN:     -r=%t2.bc,_foo,plx \
+; RUN:     -r=%t2.bc,_baz,plx \
+; RUN:     -thinlto-threads=1 \
+; RUN:     -debug-only=function-import 2>&1 | FileCheck %s
+
+; CHECK-DAG: Initialize import for 6699318081062747564 (foo)
+; CHECK-DAG: Initialize import for 15822663052811949562 (main)
+; CHECK-DAG: ref -> 7546896869197086323 (baz)
+; CHECK-DAG: edge -> 6699318081062747564 (foo) Threshold:100
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.11.0"
+
+define i32 @main() #0 {
+entry:
+  call void (...) @foo()
+  %0 = load i32, i32* @baz, align 4
+  ret i32 %0
+}
+
+declare void @foo(...) #1
+@baz = external global i32