[ThinLTO/gold] Enable summary-based internalization

Summary: Enable existing summary-based importing support in the gold-plugin.

Reviewers: mehdi_amini

Subscribers: llvm-commits, mehdi_amini

Differential Revision: http://reviews.llvm.org/D21080

llvm-svn: 272239

llvm/lib/Transforms/IPO/FunctionImport.cpp

@@ -525,8 +525,19 @@ void llvm::thinLTOInternalizeModule(Module &TheModule,
           OrigName, GlobalValue::InternalLinkage,
           TheModule.getSourceFileName());
       const auto &GS = DefinedGlobals.find(GlobalValue::getGUID(OrigId));
-      assert(GS != DefinedGlobals.end());
-      Linkage = GS->second->linkage();
+      if (GS == DefinedGlobals.end()) {
+        // Also check the original non-promoted non-globalized name. In some
+        // cases a preempted weak value is linked in as a local copy because
+        // it is referenced by an alias (IRLinker::linkGlobalValueProto).
+        // In that case, since it was originally not a local value, it was
+        // recorded in the index using the original name.
+        // FIXME: This may not be needed once PR27866 is fixed.
+        const auto &GS = DefinedGlobals.find(GlobalValue::getGUID(OrigName));
+        assert(GS != DefinedGlobals.end());
+        Linkage = GS->second->linkage();
+      } else {
+        Linkage = GS->second->linkage();
+      }
     } else
       Linkage = GS->second->linkage();
     return !GlobalValue::isLocalLinkage(Linkage);

llvm/test/tools/gold/X86/Inputs/thinlto_alias.ll

@@ -0,0 +1,4 @@
+define weak void @weakfunc() {
+entry:
+  ret void
+}

llvm/test/tools/gold/X86/Inputs/thinlto_internalize.ll

@@ -0,0 +1,6 @@
+target triple = "x86_64-unknown-linux-gnu"
+declare i32 @g()
+define i32 @main() {
+  call i32 @g()
+  ret i32 0
+}

llvm/test/tools/gold/X86/thinlto_alias.ll

@@ -0,0 +1,30 @@
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto_alias.ll -o %t2.o
+
+; Ensure that a preempted weak symbol that is linked in as a local
+; copy is handled properly. Specifically, the local copy will be promoted,
+; and internalization should be able to use the original non-promoted
+; name to locate the summary (otherwise internalization will abort because
+; it expects to locate summaries for all definitions).
+; Note that gold picks the first copy of weakfunc() as the prevailing one,
+; so listing %t2.o first is sufficient to ensure that this copy is
+; preempted.
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     --plugin-opt=thinlto \
+; RUN:     --plugin-opt=save-temps \
+; RUN:     -o %t3.o %t2.o %t.o
+; RUN: llvm-nm %t3.o | FileCheck %s
+; RUN: llvm-dis %t.o.opt.bc -o - | FileCheck --check-prefix=OPT %s
+; RUN: llvm-dis %t2.o.opt.bc -o - | FileCheck --check-prefix=OPT2 %s
+
+; CHECK-NOT: U f
+; OPT: define hidden void @weakfunc.llvm.0()
+; OPT2: define weak void @weakfunc()
+
+target triple = "x86_64-unknown-linux-gnu"
+
+@weakfuncAlias = alias void (...), bitcast (void ()* @weakfunc to void (...)*)
+define weak void @weakfunc() {
+entry:
+  ret void
+}

llvm/test/tools/gold/X86/thinlto_internalize.ll

@@ -0,0 +1,21 @@
+; RUN: opt -module-summary %s -o %t.o
+; RUN: opt -module-summary %p/Inputs/thinlto_internalize.ll -o %t2.o
+
+; RUN: %gold -m elf_x86_64 -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:     --plugin-opt=thinlto \
+; RUN:     --plugin-opt=-import-instr-limit=0 \
+; RUN:     --plugin-opt=save-temps \
+; RUN:     -o %t3.o %t2.o %t.o
+; RUN: llvm-dis %t.o.opt.bc -o - | FileCheck %s
+
+; f() should be internalized and eliminated after inlining
+; CHECK-NOT: @f()
+
+target triple = "x86_64-unknown-linux-gnu"
+define i32 @g() {
+  call void @f()
+  ret i32 0
+}
+define void @f() {
+  ret void
+}

llvm/test/tools/gold/X86/thinlto_linkonceresolution.ll

@@ -11,6 +11,7 @@
 ; RUN:     --plugin-opt=thinlto \
 ; RUN:     --plugin-opt=-import-instr-limit=0 \
 ; RUN:     --plugin-opt=save-temps \
+; RUN:     -shared \
 ; RUN:     -o %t3.o %t2.o %t.o
 ; RUN: llvm-nm %t3.o | FileCheck %s
 ; RUN: llvm-dis %t.o.opt.bc -o - | FileCheck --check-prefix=OPT %s

llvm/tools/gold/gold-plugin.cpp

@@ -897,6 +897,9 @@ class CodeGen {
   // Functions to import into this module.
   FunctionImporter::ImportMapTy *ImportList;
 
+  // Map of globals defined in this module to their summary.
+  std::map<GlobalValue::GUID, GlobalValueSummary *> *DefinedGlobals;
+
 public:
   /// Constructor used by full LTO.
   CodeGen(std::unique_ptr<llvm::Module> M)
@@ -908,10 +911,11 @@ class CodeGen {
   CodeGen(std::unique_ptr<llvm::Module> M, raw_fd_ostream *OS, int TaskID,
           const ModuleSummaryIndex *CombinedIndex, std::string Filename,
           StringMap<MemoryBufferRef> *ModuleMap,
-          FunctionImporter::ImportMapTy *ImportList)
+          FunctionImporter::ImportMapTy *ImportList,
+          std::map<GlobalValue::GUID, GlobalValueSummary *> *DefinedGlobals)
       : M(std::move(M)), OS(OS), TaskID(TaskID), CombinedIndex(CombinedIndex),
         SaveTempsFilename(std::move(Filename)), ModuleMap(ModuleMap),
-        ImportList(ImportList) {
+        ImportList(ImportList), DefinedGlobals(DefinedGlobals) {
     assert(options::thinlto == !!CombinedIndex &&
            "Expected module summary index iff performing ThinLTO");
     initTargetMachine();
@@ -996,6 +1000,14 @@ void CodeGen::runLTOPasses() {
   M->setDataLayout(TM->createDataLayout());
 
   if (CombinedIndex) {
+    // Apply summary-based internalization decisions. Skip if there are no
+    // defined globals from the summary since not only is it unnecessary, but
+    // if this module did not have a summary section the internalizer will
+    // assert if it finds any definitions in this module that aren't in the
+    // DefinedGlobals set.
+    if (!DefinedGlobals->empty())
+      thinLTOInternalizeModule(*M, *DefinedGlobals);
+
     // Create a loader that will parse the bitcode from the buffers
     // in the ModuleMap.
     ModuleLoader Loader(M->getContext(), *ModuleMap);
@@ -1137,7 +1149,7 @@ void CodeGen::runAll() {
 static void linkInModule(LLVMContext &Context, IRMover &L, claimed_file &F,
                          const void *View, StringRef Name,
                          raw_fd_ostream *ApiFile, StringSet<> &Internalize,
-                         StringSet<> &Maybe) {
+                         StringSet<> &Maybe, bool SetName = false) {
   std::vector<GlobalValue *> Keep;
   StringMap<unsigned> Realign;
   std::unique_ptr<Module> M = getModuleForFile(
@@ -1150,6 +1162,12 @@ static void linkInModule(LLVMContext &Context, IRMover &L, claimed_file &F,
     M->setTargetTriple(DefaultTriple);
   }
 
+  // For ThinLTO we want to propagate the source file name to ensure
+  // we can create the correct global identifiers matching those in the
+  // original module.
+  if (SetName)
+    L.getModule().setSourceFileName(M->getSourceFileName());
+
   if (Error E = L.move(std::move(M), Keep,
                        [](GlobalValue &, IRMover::ValueAdder) {})) {
     handleAllErrors(std::move(E), [&](const llvm::ErrorInfoBase &EIB) {
@@ -1173,7 +1191,8 @@ static void thinLTOBackendTask(claimed_file &F, const void *View,
                                const ModuleSummaryIndex &CombinedIndex,
                                raw_fd_ostream *OS, unsigned TaskID,
                                StringMap<MemoryBufferRef> &ModuleMap,
-                               FunctionImporter::ImportMapTy &ImportList) {
+                               FunctionImporter::ImportMapTy &ImportList,
+                               std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedGlobals) {
   // Need to use a separate context for each task
   LLVMContext Context;
   Context.setDiscardValueNames(options::TheOutputType !=
@@ -1185,12 +1204,12 @@ static void thinLTOBackendTask(claimed_file &F, const void *View,
   IRMover L(*NewModule.get());
 
   StringSet<> Dummy;
-  linkInModule(Context, L, F, View, Name, ApiFile, Dummy, Dummy);
+  linkInModule(Context, L, F, View, Name, ApiFile, Dummy, Dummy, true);
   if (renameModuleForThinLTO(*NewModule, CombinedIndex))
     message(LDPL_FATAL, "Failed to rename module for ThinLTO");
 
   CodeGen codeGen(std::move(NewModule), OS, TaskID, &CombinedIndex, Name,
-                  &ModuleMap, &ImportList);
+                  &ModuleMap, &ImportList, &DefinedGlobals);
   codeGen.runAll();
 }
 
@@ -1199,7 +1218,9 @@ static void
 thinLTOBackends(raw_fd_ostream *ApiFile,
                 const ModuleSummaryIndex &CombinedIndex,
                 StringMap<MemoryBufferRef> &ModuleMap,
-                StringMap<FunctionImporter::ImportMapTy> &ImportLists) {
+                StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+  StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
+      &ModuleToDefinedGVSummaries) {
   unsigned TaskCount = 0;
   std::vector<ThinLTOTaskInfo> Tasks;
   Tasks.reserve(Modules.size());
@@ -1243,7 +1264,8 @@ thinLTOBackends(raw_fd_ostream *ApiFile,
       ThinLTOThreadPool.async(thinLTOBackendTask, std::ref(F), View, F.name,
                               ApiFile, std::ref(CombinedIndex), OS.get(),
                               TaskCount, std::ref(ModuleMap),
-                              std::ref(ImportLists[F.name]));
+                              std::ref(ImportLists[F.name]),
+                              std::ref(ModuleToDefinedGVSummaries[F.name]));
 
       // Record the information needed by the task or during its cleanup
       // to a ThinLTOTaskInfo instance. For information needed by the task
@@ -1298,6 +1320,11 @@ static ld_plugin_status thinLTOLink(raw_fd_ostream *ApiFile) {
   // interfaces with gold.
   DenseMap<void *, std::unique_ptr<PluginInputFile>> HandleToInputFile;
 
+  // Keep track of internalization candidates as well as those that may not
+  // be internalized because they are refereneced from other IR modules.
+  DenseSet<GlobalValue::GUID> Internalize;
+  DenseSet<GlobalValue::GUID> CrossReferenced;
+
   ModuleSummaryIndex CombinedIndex;
   uint64_t NextModuleId = 0;
   for (claimed_file &F : Modules) {
@@ -1321,11 +1348,23 @@ static ld_plugin_status thinLTOLink(raw_fd_ostream *ApiFile) {
     // Skip files without a module summary.
     if (Index)
       CombinedIndex.mergeFrom(std::move(Index), ++NextModuleId);
+
+    // Look for internalization candidates based on gold's symbol resolution
+    // information. Also track symbols referenced from other IR modules.
+    for (auto &Sym : F.syms) {
+      ld_plugin_symbol_resolution Resolution =
+          (ld_plugin_symbol_resolution)Sym.resolution;
+      if (Resolution == LDPR_PREVAILING_DEF_IRONLY)
+        Internalize.insert(GlobalValue::getGUID(Sym.name));
+      if (Resolution == LDPR_RESOLVED_IR || Resolution == LDPR_PREEMPTED_IR)
+        CrossReferenced.insert(GlobalValue::getGUID(Sym.name));
+    }
   }
 
-  if (options::thinlto_emit_imports_files && !options::thinlto_index_only)
-    message(LDPL_WARNING,
-            "thinlto-emit-imports-files ignored unless thinlto-index-only");
+  // Remove symbols referenced from other IR modules from the internalization
+  // candidate set.
+  for (auto &S : CrossReferenced)
+    Internalize.erase(S);
 
   // Collect for each module the list of function it defines (GUID ->
   // Summary).
@@ -1338,6 +1377,25 @@ static ld_plugin_status thinLTOLink(raw_fd_ostream *ApiFile) {
   ComputeCrossModuleImport(CombinedIndex, ModuleToDefinedGVSummaries,
                            ImportLists, ExportLists);
 
+  // Callback for internalization, to prevent internalization of symbols
+  // that were not candidates initially, and those that are being imported
+  // (which introduces new cross references).
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    const auto &ExportList = ExportLists.find(ModuleIdentifier);
+    return (ExportList != ExportLists.end() &&
+            ExportList->second.count(GUID)) ||
+           !Internalize.count(GUID);
+  };
+
+  // Use global summary-based analysis to identify symbols that can be
+  // internalized (because they aren't exported or preserved as per callback).
+  // Changes are made in the index, consumed in the ThinLTO backends.
+  thinLTOInternalizeAndPromoteInIndex(CombinedIndex, isExported);
+
+  if (options::thinlto_emit_imports_files && !options::thinlto_index_only)
+    message(LDPL_WARNING,
+            "thinlto-emit-imports-files ignored unless thinlto-index-only");
+
   if (options::thinlto_index_only) {
     // If the thinlto-prefix-replace option was specified, parse it and
     // extract the old and new prefixes.
@@ -1388,7 +1446,8 @@ static ld_plugin_status thinLTOLink(raw_fd_ostream *ApiFile) {
     WriteIndexToFile(CombinedIndex, OS);
   }
 
-  thinLTOBackends(ApiFile, CombinedIndex, ModuleMap, ImportLists);
+  thinLTOBackends(ApiFile, CombinedIndex, ModuleMap, ImportLists,
+                  ModuleToDefinedGVSummaries);
   return LDPS_OK;
 }