Index: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp =================================================================== --- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp +++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp @@ -525,8 +525,19 @@ 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); Index: llvm/trunk/test/tools/gold/X86/Inputs/thinlto_alias.ll =================================================================== --- llvm/trunk/test/tools/gold/X86/Inputs/thinlto_alias.ll +++ llvm/trunk/test/tools/gold/X86/Inputs/thinlto_alias.ll @@ -0,0 +1,4 @@ +define weak void @weakfunc() { +entry: + ret void +} Index: llvm/trunk/test/tools/gold/X86/Inputs/thinlto_internalize.ll =================================================================== --- llvm/trunk/test/tools/gold/X86/Inputs/thinlto_internalize.ll +++ llvm/trunk/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 +} Index: llvm/trunk/test/tools/gold/X86/thinlto_alias.ll =================================================================== --- llvm/trunk/test/tools/gold/X86/thinlto_alias.ll +++ llvm/trunk/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 +} Index: llvm/trunk/test/tools/gold/X86/thinlto_internalize.ll =================================================================== --- llvm/trunk/test/tools/gold/X86/thinlto_internalize.ll +++ llvm/trunk/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 +} Index: llvm/trunk/test/tools/gold/X86/thinlto_linkonceresolution.ll =================================================================== --- llvm/trunk/test/tools/gold/X86/thinlto_linkonceresolution.ll +++ llvm/trunk/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 Index: llvm/trunk/tools/gold/gold-plugin.cpp =================================================================== --- llvm/trunk/tools/gold/gold-plugin.cpp +++ llvm/trunk/tools/gold/gold-plugin.cpp @@ -897,6 +897,9 @@ // Functions to import into this module. FunctionImporter::ImportMapTy *ImportList; + // Map of globals defined in this module to their summary. + std::map *DefinedGlobals; + public: /// Constructor used by full LTO. CodeGen(std::unique_ptr M) @@ -908,10 +911,11 @@ CodeGen(std::unique_ptr M, raw_fd_ostream *OS, int TaskID, const ModuleSummaryIndex *CombinedIndex, std::string Filename, StringMap *ModuleMap, - FunctionImporter::ImportMapTy *ImportList) + FunctionImporter::ImportMapTy *ImportList, + std::map *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 @@ 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 @@ 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 Keep; StringMap Realign; std::unique_ptr M = getModuleForFile( @@ -1150,6 +1162,12 @@ 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 @@ const ModuleSummaryIndex &CombinedIndex, raw_fd_ostream *OS, unsigned TaskID, StringMap &ModuleMap, - FunctionImporter::ImportMapTy &ImportList) { + FunctionImporter::ImportMapTy &ImportList, + std::map &DefinedGlobals) { // Need to use a separate context for each task LLVMContext Context; Context.setDiscardValueNames(options::TheOutputType != @@ -1185,12 +1204,12 @@ 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 @@ thinLTOBackends(raw_fd_ostream *ApiFile, const ModuleSummaryIndex &CombinedIndex, StringMap &ModuleMap, - StringMap &ImportLists) { + StringMap &ImportLists, + StringMap> + &ModuleToDefinedGVSummaries) { unsigned TaskCount = 0; std::vector Tasks; Tasks.reserve(Modules.size()); @@ -1243,7 +1264,8 @@ 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 @@ // interfaces with gold. DenseMap> HandleToInputFile; + // Keep track of internalization candidates as well as those that may not + // be internalized because they are refereneced from other IR modules. + DenseSet Internalize; + DenseSet CrossReferenced; + ModuleSummaryIndex CombinedIndex; uint64_t NextModuleId = 0; for (claimed_file &F : Modules) { @@ -1321,11 +1348,23 @@ // 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 @@ 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 @@ WriteIndexToFile(CombinedIndex, OS); } - thinLTOBackends(ApiFile, CombinedIndex, ModuleMap, ImportLists); + thinLTOBackends(ApiFile, CombinedIndex, ModuleMap, ImportLists, + ModuleToDefinedGVSummaries); return LDPS_OK; }