[ThinLTO] Add optional import message and statistics

[tejohnson]

Summary

Add a statistic to count the number of imported functions. Also, add a
new -print-imports option to emit a trace of imported functions, that
works even for an NDEBUG build.

Note that emitOptimizationRemark does not work for the above printing as
it expects a Function object and DebugLoc, neither of which we will have
particularly when we move to summary-based importing.

[mehdi_amini]

Do you mind rebasing on http://reviews.llvm.org/D18343 ?

[tejohnson]

Rebase on D18343/r264503.

Also, with r264503, aliases are now being added to the GlobalsToImport set even
when their aliasees can't be imported due to their linkage type. While the
importing worked correctly (the aliases imported as declarations) due to the
logic in FunctionImportGlobalProcessing::doImportAsDefinition, there is no
point to adding them to the GlobalsToImport set and it was resulting in
incorrectly printing a message indicating that the alias was imported.
To avoid this, delay adding aliases to the GlobalsToImport set until after
the linkage type of the aliasee is checked.

[mehdi_amini]

LGTM, but please commit separately if possible: first the change about the aliases and then the changes related to the new debug.

[aganea]

llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

359 ↗

(On Diff #51736)

Hello Teresa! This debug output is not thread-safe, but it is executed from ThinLTO threads. There's a test, llvm/test/Transforms/PGOProfile/Inputs/thinlto_samplepgo_icp3.ll which relies on this output. Currently that test only uses one thread (-thinlto-threads=1). However when using any other number of threads, the test randomly fails.
I wanted to test other values for -thinlto-threads in D75153, but I cannot. I'd like to land that patch, and would like your advice.

What avenue would you suggest? Add/use a mutex here? Remove the coverage for thinlto_samplepgo_icp3.ll in D75153?

[tejohnson]

llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

359 ↗

(On Diff #51736)

I don't think it is a problem unique to this particular debugging output. Any ThinLTO test that uses a -debug or -debug-only to get messages from backend threads will hit it as well if there is more than one thread.

I'd probably either remove the changes from that test, or better, use something other than the -print-inputs flag to test that the importing happened. I.e. check the llvm-dis on the -save-temps .import.bc file.

Actually,. that test looks like it has an issue - there is a line to test with the "; ICALL-PROM:" prefix, which is not ever actually invoked. Presumably could be fixed by also checking the llvm-dis of the temp file.

[aganea]

llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp

359 ↗

(On Diff #51736)

Just for the record, @tejohnson 's suggestion was implemented in rG934d4feab1f58ede295a3232ac47f3c01c9fc506.

lib/Transforms/IPO/FunctionImport.cpp

 //===- FunctionImport.cpp - ThinLTO Summary-based Function Import ---------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements Function import based on summaries.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/IPO/FunctionImport.h"
 
+#include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Object/ModuleSummaryIndexObjectFile.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 
 #include <map>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "function-import"
 
+STATISTIC(NumImported, "Number of functions imported");
+
 /// Limit on instruction count of imported functions.
 static cl::opt<unsigned> ImportInstrLimit(
     "import-instr-limit", cl::init(100), cl::Hidden, cl::value_desc("N"),
     cl::desc("Only import functions with less than N instructions"));
 
 static cl::opt<float>
     ImportInstrFactor("import-instr-evolution-factor", cl::init(0.7),
                       cl::Hidden, cl::value_desc("x"),
                       cl::desc("As we import functions, multiply the "
                                "`import-instr-limit` threshold by this factor "
                                "before processing newly imported functions"));
 
+static cl::opt<bool> PrintImports("print-imports", cl::init(false), cl::Hidden,
+                                  cl::desc("Print imported functions"));
+
 // Load lazily a module from \p FileName in \p Context.
 static std::unique_ptr<Module> loadFile(const std::string &FileName,
                                         LLVMContext &Context) {
   SMDiagnostic Err;
   DEBUG(dbgs() << "Loading '" << FileName << "'\n");
   // Metadata isn't loaded until functions are imported, to minimize
   // the memory overhead.
   std::unique_ptr<Module> Result =
if (SGV->hasWeakAnyLinkage()) {
                    << SrcModule.getModuleIdentifier() << "\n");
       continue;
     }
 
     // Add the function to the import list
     auto &Entry = ModuleToFunctionsToImportMap[SrcModule.getModuleIdentifier()];
     Entry.insert(F);
 
+    if (PrintImports)
+      dbgs() << DestModule.getSourceFileName() << ": Import " << F->getName()
+             << " from " << SrcModule.getModuleIdentifier() << "\n";
+
     // Process the newly imported functions and add callees to the worklist.
     // Adjust the threshold
     Threshold = Threshold * ImportInstrFactor;
     F->materialize();
     findExternalCalls(DestModule, *F, Index, VisitedFunctions, Threshold,
                       Worklist);
   }
 }
for (auto &FunctionsToImportPerModule : ModuleToFunctionsToImportMap) {
 
     if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None,
                                &FunctionsToImport))
       report_fatal_error("Function Import: link error");
 
     ImportedCount += FunctionsToImport.size();
   }
 
+  NumImported += ImportedCount;
+
   DEBUG(dbgs() << "Imported " << ImportedCount << " functions for Module "
                << DestModule.getModuleIdentifier() << "\n");
   return ImportedCount;
 }
 
 /// Summary file to use for function importing when using -function-import from
 /// the command line.
 static cl::opt<std::string>

test/Transforms/FunctionImport/funcimport.ll

 ; Do setup work for all below tests: generate bitcode and combined index
 ; RUN: llvm-as -module-summary %s -o %t.bc
 ; RUN: llvm-as -module-summary %p/Inputs/funcimport.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
 
 ; Do the import now
-; RUN: opt -function-import -summary-file %t3.thinlto.bc %s -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
+; RUN: opt -function-import -stats -print-imports -summary-file %t3.thinlto.bc %s -S 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIMDEF
 
 ; Test import with smaller instruction limit
 ; RUN: opt -function-import -summary-file %t3.thinlto.bc %s -import-instr-limit=5 -S | FileCheck %s --check-prefix=CHECK --check-prefix=INSTLIM5
 ; INSTLIM5-NOT: @staticfunc.llvm.2
 
 define i32 @main() #0 {
 entry:
   call void (...) @weakalias()

 declare void @weakalias(...) #1
 
 ; Cannot create an alias to available_externally
 ; CHECK-DAG: declare void @analias
 declare void @analias(...) #1
 
 ; Aliases import the aliasee function
 declare void @linkoncealias(...) #1
+; INSTLIMDEF-DAG: Import linkoncefunc
 ; CHECK-DAG: define linkonce_odr void @linkoncefunc()
 ; CHECK-DAG: @linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*
 
+; INSTLIMDEF-DAG: Import referencestatics
 ; INSTLIMDEF-DAG: define available_externally i32 @referencestatics(i32 %i)
 ; INSTLIM5-DAG: declare i32 @referencestatics(...)
 declare i32 @referencestatics(...) #1
 
 ; The import of referencestatics will expose call to staticfunc that
 ; should in turn be imported as a promoted/renamed and hidden function.
 ; Ensure that the call is to the properly-renamed function.
+; INSTLIMDEF-DAG: Import staticfunc
 ; INSTLIMDEF-DAG: %call = call i32 @staticfunc.llvm.2()
 ; INSTLIMDEF-DAG: define available_externally hidden i32 @staticfunc.llvm.2()
 
+; INSTLIMDEF-DAG: Import referenceglobals
 ; CHECK-DAG: define available_externally i32 @referenceglobals(i32 %i)
 declare i32 @referenceglobals(...) #1
 
 ; The import of referenceglobals will expose call to globalfunc1 that
 ; should in turn be imported.
+; INSTLIMDEF-DAG: Import globalfunc1
 ; CHECK-DAG: define available_externally void @globalfunc1()
 
+; INSTLIMDEF-DAG: Import referencecommon
 ; CHECK-DAG: define available_externally i32 @referencecommon(i32 %i)
 declare i32 @referencecommon(...) #1
 
+; INSTLIMDEF-DAG: Import setfuncptr
 ; CHECK-DAG: define available_externally void @setfuncptr()
 declare void @setfuncptr(...) #1
 
+; INSTLIMDEF-DAG: Import callfuncptr
 ; CHECK-DAG: define available_externally void @callfuncptr()
 declare void @callfuncptr(...) #1
 
 ; Ensure that all uses of local variable @P which has used in setfuncptr
 ; and callfuncptr are to the same promoted/renamed global.
 ; CHECK-DAG: @P.llvm.2 = external hidden global void ()*
 ; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.2,
 ; CHECK-DAG: store void ()* @staticfunc2.llvm.2, void ()** @P.llvm.2,
 
 ; Won't import weak func
 ; CHECK-DAG: declare void @weakfunc(...)
 declare void @weakfunc(...) #1
 
+; INSTLIMDEF-DAG: Import funcwithpersonality
 ; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.2() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 ; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.2()
+
+; INSTLIMDEF-DAG: Import globalfunc2
+; INSTLIMDEF: 10 function-import - Number of functions imported