Index: include/llvm/IR/ModuleSummaryIndex.h =================================================================== --- include/llvm/IR/ModuleSummaryIndex.h +++ include/llvm/IR/ModuleSummaryIndex.h @@ -250,6 +250,7 @@ friend class ModuleSummaryIndex; friend void computeDeadSymbols(class ModuleSummaryIndex &, + const DenseSet &, const DenseSet &); }; Index: include/llvm/Transforms/IPO/FunctionImport.h =================================================================== --- include/llvm/Transforms/IPO/FunctionImport.h +++ include/llvm/Transforms/IPO/FunctionImport.h @@ -112,7 +112,10 @@ /// \p GUIDPreservedSymbols. void computeDeadSymbols( ModuleSummaryIndex &Index, - const DenseSet &GUIDPreservedSymbols); + const DenseSet &GUIDPreservedSymbols, + const DenseSet &GUIDPrevailingSymbols); + +void convertToDeclaration(GlobalValue &GV); /// Compute the set of summaries needed for a ThinLTO backend compilation of /// \p ModulePath. Index: lib/LTO/LTO.cpp =================================================================== --- lib/LTO/LTO.cpp +++ lib/LTO/LTO.cpp @@ -745,16 +745,21 @@ Error LTO::run(AddStreamFn AddStream, NativeObjectCache Cache) { // Compute "dead" symbols, we don't want to import/export these! DenseSet GUIDPreservedSymbols; + DenseSet GUIDPrevailingSymbols; for (auto &Res : GlobalResolutions) { - if (Res.second.VisibleOutsideSummary && - // IRName will be defined if we have seen the prevailing copy of - // this value. If not, no need to preserve any ThinLTO copies. - !Res.second.IRName.empty()) - GUIDPreservedSymbols.insert(GlobalValue::getGUID( - GlobalValue::dropLLVMManglingEscape(Res.second.IRName))); - } + // IRName will be defined if we have seen the prevailing copy of + // this value. If not, no need to preserve any ThinLTO copies. + if (Res.second.IRName.empty()) + continue; - computeDeadSymbols(ThinLTO.CombinedIndex, GUIDPreservedSymbols); + GlobalValue::GUID G = GlobalValue::getGUID( + GlobalValue::dropLLVMManglingEscape(Res.second.IRName)); + GUIDPrevailingSymbols.insert(G); + if (Res.second.VisibleOutsideSummary) + GUIDPreservedSymbols.insert(G); + } + computeDeadSymbols(ThinLTO.CombinedIndex, GUIDPreservedSymbols, + GUIDPrevailingSymbols); if (auto E = runRegularLTO(AddStream)) return E; Index: lib/LTO/LTOBackend.cpp =================================================================== --- lib/LTO/LTOBackend.cpp +++ lib/LTO/LTOBackend.cpp @@ -393,6 +393,19 @@ return Error::success(); } +static void dropDeadSymbols(Module &Mod, const GVSummaryMapTy &DefinedGlobals, + const ModuleSummaryIndex &Index) { + for (auto &GV : Mod) { + const auto &GS = DefinedGlobals.find(GV.getGUID()); + if (GS == DefinedGlobals.end()) + continue; + if (Index.isGlobalValueLive(GS->second)) + continue; + + convertToDeclaration(GV); + } +} + Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream, Module &Mod, const ModuleSummaryIndex &CombinedIndex, const FunctionImporter::ImportMapTy &ImportList, @@ -414,6 +427,8 @@ renameModuleForThinLTO(Mod, CombinedIndex); + dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex); + thinLTOResolveWeakForLinkerModule(Mod, DefinedGlobals); if (Conf.PostPromoteModuleHook && !Conf.PostPromoteModuleHook(Task, Mod)) Index: lib/LTO/ThinLTOCodeGenerator.cpp =================================================================== --- lib/LTO/ThinLTOCodeGenerator.cpp +++ lib/LTO/ThinLTOCodeGenerator.cpp @@ -621,6 +621,18 @@ thinLTOInternalizeAndPromoteInIndex(Index, isExported); } +static void markLive(ModuleSummaryIndex &Index, + const DenseSet &GUIDPreservedSymbols) { + DenseMap PrevailingCopy; + computePrevailingCopies(Index, PrevailingCopy); + + DenseSet GUIDPrevailingSymbols; + for (auto &I : PrevailingCopy) + GUIDPrevailingSymbols.insert(I.first); + + computeDeadSymbols(Index, GUIDPreservedSymbols, GUIDPrevailingSymbols); +} + /** * Perform promotion and renaming of exported internal functions. * Index is updated to reflect linkage changes from weak resolution. @@ -639,7 +651,7 @@ PreservedSymbols, Triple(TheModule.getTargetTriple())); // Compute "dead" symbols, we don't want to import/export these! - computeDeadSymbols(Index, GUIDPreservedSymbols); + markLive(Index, GUIDPreservedSymbols); // Generate import/export list StringMap ImportLists(ModuleCount); @@ -678,7 +690,7 @@ PreservedSymbols, Triple(TheModule.getTargetTriple())); // Compute "dead" symbols, we don't want to import/export these! - computeDeadSymbols(Index, GUIDPreservedSymbols); + markLive(Index, GUIDPreservedSymbols); // Generate import/export list StringMap ImportLists(ModuleCount); @@ -755,7 +767,7 @@ Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries); // Compute "dead" symbols, we don't want to import/export these! - computeDeadSymbols(Index, GUIDPreservedSymbols); + markLive(Index, GUIDPreservedSymbols); // Generate import/export list StringMap ImportLists(ModuleCount); @@ -901,7 +913,7 @@ computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple); // Compute "dead" symbols, we don't want to import/export these! - computeDeadSymbols(*Index, GUIDPreservedSymbols); + markLive(*Index, GUIDPreservedSymbols); // Collect the import/export lists for all modules from the call-graph in the // combined index. Index: lib/Transforms/IPO/FunctionImport.cpp =================================================================== --- lib/Transforms/IPO/FunctionImport.cpp +++ lib/Transforms/IPO/FunctionImport.cpp @@ -178,8 +178,11 @@ // filtered out. if (GVSummary->getSummaryKind() == GlobalValueSummary::GlobalVarKind) return false; + + // There is no point in importing these, we can't inline them if (GlobalValue::isInterposableLinkage(GVSummary->linkage())) - // There is no point in importing these, we can't inline them + return false; + if (!Index.isGlobalValueLive(GVSummary)) return false; auto *Summary = cast(GVSummary->getBaseObject()); @@ -494,7 +497,8 @@ void llvm::computeDeadSymbols( ModuleSummaryIndex &Index, - const DenseSet &GUIDPreservedSymbols) { + const DenseSet &GUIDPreservedSymbols, + const DenseSet &GUIDPrevailingSymbols) { assert(!Index.withGlobalValueDeadStripping()); if (!ComputeDead) return; @@ -523,7 +527,6 @@ } // Make value live and add it to the worklist if it was not live before. - // FIXME: we should only make the prevailing copy live here 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 @@ -536,6 +539,8 @@ VI = updateValueInfoForIndirectCalls(Index, VI); if (!VI) return; + if (!GUIDPrevailingSymbols.count(VI.getGUID())) + return; for (auto &S : VI.getSummaryList()) if (S->isLive()) return; @@ -602,26 +607,26 @@ return std::error_code(); } +void llvm::convertToDeclaration(GlobalValue &GV) { + DEBUG(dbgs() << "Converting to a declaration: `" << GV.getName() << "\n"); + if (Function *F = dyn_cast(&GV)) { + F->deleteBody(); + F->clearMetadata(); + } else if (GlobalVariable *V = dyn_cast(&GV)) { + V->setInitializer(nullptr); + V->setLinkage(GlobalValue::ExternalLinkage); + V->clearMetadata(); + } else + // For now we don't resolve or drop aliases. Once we do we'll + // need to add support here for creating either a function or + // variable declaration, and return the new GlobalValue* for + // the caller to use. + llvm_unreachable("Expected function or variable"); +} + /// Fixup WeakForLinker linkages in \p TheModule based on summary analysis. void llvm::thinLTOResolveWeakForLinkerModule( Module &TheModule, const GVSummaryMapTy &DefinedGlobals) { - auto ConvertToDeclaration = [](GlobalValue &GV) { - DEBUG(dbgs() << "Converting to a declaration: `" << GV.getName() << "\n"); - if (Function *F = dyn_cast(&GV)) { - F->deleteBody(); - F->clearMetadata(); - } else if (GlobalVariable *V = dyn_cast(&GV)) { - V->setInitializer(nullptr); - V->setLinkage(GlobalValue::ExternalLinkage); - V->clearMetadata(); - } else - // For now we don't resolve or drop aliases. Once we do we'll - // need to add support here for creating either a function or - // variable declaration, and return the new GlobalValue* for - // the caller to use. - llvm_unreachable("Expected function or variable"); - }; - auto updateLinkage = [&](GlobalValue &GV) { // See if the global summary analysis computed a new resolved linkage. const auto &GS = DefinedGlobals.find(GV.getGUID()); @@ -652,7 +657,7 @@ // the definition in that case. if (GlobalValue::isAvailableExternallyLinkage(NewLinkage) && GlobalValue::isInterposableLinkage(GV.getLinkage())) - ConvertToDeclaration(GV); + convertToDeclaration(GV); else { DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from " << GV.getLinkage() << " to " << NewLinkage << "\n"); Index: test/LTO/Resolution/X86/Inputs/not-prevailing.ll =================================================================== --- test/LTO/Resolution/X86/Inputs/not-prevailing.ll +++ test/LTO/Resolution/X86/Inputs/not-prevailing.ll @@ -0,0 +1,6 @@ +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define void @bar() { + ret void +} Index: test/LTO/Resolution/X86/not-prevailing.ll =================================================================== --- test/LTO/Resolution/X86/not-prevailing.ll +++ test/LTO/Resolution/X86/not-prevailing.ll @@ -0,0 +1,37 @@ +; RUN: opt -module-summary %s -o %t1.o +; RUN: opt -module-summary -o %t2.o %S/Inputs/not-prevailing.ll +; RUN: llvm-lto2 run -o %t3.o %t1.o %t2.o -r %t1.o,foo,x -r %t1.o,zed,px -r %t1.o,bar,x \ +; RUN: -r %t2.o,bar,x -save-temps + +; Check that 'foo' and 'bar' were not inlined. +; CHECK: zed: +; CHECK-NEXT: {{.*}} pushq %rbx +; CHECK-NEXT: {{.*}} callq 0 +; CHECK-NEXT: {{.*}} movl %eax, %ebx +; CHECK-NEXT: {{.*}} callq 0 +; CHECK-NEXT: {{.*}} movl %ebx, %eax +; CHECK-NEXT: {{.*}} popq %rbx +; CHECK-NEXT: {{.*}} retq + +; RUN: llvm-objdump -d %t3.o.1 | FileCheck %s +; RUN: llvm-readelf --symbols %t3.o.1 | FileCheck %s --check-prefix=SYMBOLS + +; Check that 'foo' and 'bar' produced as undefined. +; SYMBOLS: NOTYPE GLOBAL DEFAULT UND bar +; SYMBOLS: NOTYPE GLOBAL DEFAULT UND foo +; SYMBOLS: FUNC GLOBAL DEFAULT 2 zed + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define weak i32 @foo() { + ret i32 65 +} + +declare void @bar() + +define i32 @zed() { + %1 = tail call i32 @foo() + call void @bar() + ret i32 %1 +}