From 438900938c3ac9d7fac2dd5d2c85ca4b9b2e35f7 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Thu, 3 Oct 2013 18:29:09 +0000 Subject: [PATCH] Optimize linkonce_odr unnamed_addr functions during LTO. Generalize the API so we can distinguish symbols that are needed just for a DSO symbol table from those that are used from some native .o. The symbols that are only wanted for the dso symbol table can be dropped if llvm can prove every other dso has a copy (linkonce_odr) and the address is not important (unnamed_addr). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@191922 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm-c/lto.h | 16 ++++++-- include/llvm/LTO/LTOCodeGenerator.h | 6 +++ include/llvm/Transforms/IPO.h | 26 +++++++++++-- lib/LTO/LTOCodeGenerator.cpp | 12 ++++-- lib/Transforms/IPO/IPO.cpp | 2 +- lib/Transforms/IPO/Internalize.cpp | 46 ++++++++++++++++++----- lib/Transforms/IPO/PassManagerBuilder.cpp | 2 +- test/LTO/cfi_endproc.ll | 8 ++++ test/Transforms/Internalize/lists.ll | 10 +++++ tools/gold/gold-plugin.cpp | 7 +++- tools/llvm-lto/llvm-lto.cpp | 8 ++++ tools/lto/lto.cpp | 4 ++ 12 files changed, 124 insertions(+), 23 deletions(-) diff --git a/include/llvm-c/lto.h b/include/llvm-c/lto.h index 58d58336b50..85d5439ec94 100644 --- a/include/llvm-c/lto.h +++ b/include/llvm-c/lto.h @@ -29,7 +29,7 @@ * @{ */ -#define LTO_API_VERSION 4 +#define LTO_API_VERSION 5 typedef enum { LTO_SYMBOL_ALIGNMENT_MASK = 0x0000001F, /* log2 of alignment */ @@ -253,13 +253,21 @@ lto_codegen_set_assembler_args(lto_code_gen_t cg, const char **args, int nargs); /** - * Adds to a list of all global symbols that must exist in the final - * generated code. If a function is not listed, it might be - * inlined into every usage and optimized away. + * Tells LTO optimization passes that this symbol must be preserved + * because it is referenced by native code or a command line option. */ extern void lto_codegen_add_must_preserve_symbol(lto_code_gen_t cg, const char* symbol); + +/** + * Tells LTO optimization passes that a dynamic shared library is being + * built and this symbol may be exported. Unless IR semantics allow the symbol + * to be made local to the library, it should remain so it can be exported by + * the shared library. + */ +extern void lto_codegen_add_dso_symbol(lto_code_gen_t cg, const char *symbol); + /** * Writes a new object file at the specified path that contains the * merged contents of all modules added so far. diff --git a/include/llvm/LTO/LTOCodeGenerator.h b/include/llvm/LTO/LTOCodeGenerator.h index 97a50663398..9f50770dcd9 100644 --- a/include/llvm/LTO/LTOCodeGenerator.h +++ b/include/llvm/LTO/LTOCodeGenerator.h @@ -73,6 +73,10 @@ struct LTOCodeGenerator { void addMustPreserveSymbol(const char *sym) { MustPreserveSymbols[sym] = 1; } + void addDSOSymbol(const char* Sym) { + DSOSymbols[Sym] = 1; + } + // To pass options to the driver and optimization passes. These options are // not necessarily for debugging purpose (The function name is misleading). // This function should be called before LTOCodeGenerator::compilexxx(), @@ -126,6 +130,7 @@ private: void applyScopeRestrictions(); void applyRestriction(llvm::GlobalValue &GV, std::vector &MustPreserveList, + std::vector &SymtabList, llvm::SmallPtrSet &AsmUsed, llvm::Mangler &Mangler); bool determineTarget(std::string &errMsg); @@ -138,6 +143,7 @@ private: bool EmitDwarfDebugInfo; bool ScopeRestrictionsDone; lto_codegen_model CodeModel; + StringSet DSOSymbols; StringSet MustPreserveSymbols; StringSet AsmUndefinedRefs; llvm::MemoryBuffer *NativeObjectFile; diff --git a/include/llvm/Transforms/IPO.h b/include/llvm/Transforms/IPO.h index 6ebb6c46586..ed2823098ae 100644 --- a/include/llvm/Transforms/IPO.h +++ b/include/llvm/Transforms/IPO.h @@ -104,12 +104,32 @@ Pass *createPruneEHPass(); //===----------------------------------------------------------------------===// /// createInternalizePass - This pass loops over all of the functions in the -/// input module, internalizing all globals (functions and variables) not in the -/// given exportList. +/// input module, internalizing all globals (functions and variables) it can. +//// +/// The symbols in \p ExportList are never internalized. +/// +/// The symbol in DSOList are internalized if it is safe to drop them from +/// the symbol table. +/// +/// For example of the difference, consider a dynamic library being built from +/// two translation units. The first one compiled to a native object +/// (ELF/MachO/COFF) and second one compiled to IL. Translation unit A has a +/// copy of linkonce_odr unnamed_addr function F. The translation unit B has a +/// copy of the linkonce_odr unnamed_addr functions F and G. +/// +/// Assume the linker decides to keep the copy of F in B. This means that LLVM +/// must produce F in the object file it passes to the linker, otherwise we +/// will have an undefined reference. For G the situation is different. The +/// linker puts the function in the DSOList, since it is only wanted for the +/// symbol table. With this information internalize can now reason that since +/// the function is a linkonce_odr and its address is not important, it can be +/// omitted. Any other shared library needing this function will have a copy of +/// it. /// /// Note that commandline options that are used with the above function are not /// used now! -ModulePass *createInternalizePass(ArrayRef ExportList); +ModulePass *createInternalizePass(ArrayRef ExportList, + ArrayRef DSOList); /// createInternalizePass - Same as above, but with an empty exportList. ModulePass *createInternalizePass(); diff --git a/lib/LTO/LTOCodeGenerator.cpp b/lib/LTO/LTOCodeGenerator.cpp index 3abb6238b3e..e35e33666ec 100644 --- a/lib/LTO/LTOCodeGenerator.cpp +++ b/lib/LTO/LTOCodeGenerator.cpp @@ -310,6 +310,7 @@ bool LTOCodeGenerator::determineTarget(std::string &errMsg) { void LTOCodeGenerator:: applyRestriction(GlobalValue &GV, std::vector &MustPreserveList, + std::vector &DSOList, SmallPtrSet &AsmUsed, Mangler &Mangler) { SmallString<64> Buffer; @@ -319,6 +320,8 @@ applyRestriction(GlobalValue &GV, return; if (MustPreserveSymbols.count(Buffer)) MustPreserveList.push_back(GV.getName().data()); + if (DSOSymbols.count(Buffer)) + DSOList.push_back(GV.getName().data()); if (AsmUndefinedRefs.count(Buffer)) AsmUsed.insert(&GV); } @@ -348,17 +351,18 @@ void LTOCodeGenerator::applyScopeRestrictions() { NULL); Mangler Mangler(MContext, TargetMach); std::vector MustPreserveList; + std::vector DSOList; SmallPtrSet AsmUsed; for (Module::iterator f = mergedModule->begin(), e = mergedModule->end(); f != e; ++f) - applyRestriction(*f, MustPreserveList, AsmUsed, Mangler); + applyRestriction(*f, MustPreserveList, DSOList, AsmUsed, Mangler); for (Module::global_iterator v = mergedModule->global_begin(), e = mergedModule->global_end(); v != e; ++v) - applyRestriction(*v, MustPreserveList, AsmUsed, Mangler); + applyRestriction(*v, MustPreserveList, DSOList, AsmUsed, Mangler); for (Module::alias_iterator a = mergedModule->alias_begin(), e = mergedModule->alias_end(); a != e; ++a) - applyRestriction(*a, MustPreserveList, AsmUsed, Mangler); + applyRestriction(*a, MustPreserveList, DSOList, AsmUsed, Mangler); GlobalVariable *LLVMCompilerUsed = mergedModule->getGlobalVariable("llvm.compiler.used"); @@ -386,7 +390,7 @@ void LTOCodeGenerator::applyScopeRestrictions() { LLVMCompilerUsed->setSection("llvm.metadata"); } - passes.add(createInternalizePass(MustPreserveList)); + passes.add(createInternalizePass(MustPreserveList, DSOList)); // apply scope restrictions passes.run(*mergedModule); diff --git a/lib/Transforms/IPO/IPO.cpp b/lib/Transforms/IPO/IPO.cpp index 5d563d8bbf5..5f26bac2c8f 100644 --- a/lib/Transforms/IPO/IPO.cpp +++ b/lib/Transforms/IPO/IPO.cpp @@ -98,7 +98,7 @@ void LLVMAddInternalizePass(LLVMPassManagerRef PM, unsigned AllButMain) { std::vector Export; if (AllButMain) Export.push_back("main"); - unwrap(PM)->add(createInternalizePass(Export)); + unwrap(PM)->add(createInternalizePass(Export, None)); } void LLVMAddStripDeadPrototypesPass(LLVMPassManagerRef PM) { diff --git a/lib/Transforms/IPO/Internalize.cpp b/lib/Transforms/IPO/Internalize.cpp index f2feacc0b63..f20a7bd92f4 100644 --- a/lib/Transforms/IPO/Internalize.cpp +++ b/lib/Transforms/IPO/Internalize.cpp @@ -44,13 +44,20 @@ APIList("internalize-public-api-list", cl::value_desc("list"), cl::desc("A list of symbol names to preserve"), cl::CommaSeparated); +static cl::list +DSOList("internalize-dso-list", cl::value_desc("list"), + cl::desc("A list of symbol names need for a dso symbol table"), + cl::CommaSeparated); + namespace { class InternalizePass : public ModulePass { std::set ExternalNames; + std::set DSONames; public: static char ID; // Pass identification, replacement for typeid explicit InternalizePass(); - explicit InternalizePass(ArrayRef ExportList); + explicit InternalizePass(ArrayRef ExportList, + ArrayRef DSOList); void LoadFile(const char *Filename); virtual bool runOnModule(Module &M); @@ -71,15 +78,21 @@ InternalizePass::InternalizePass() if (!APIFile.empty()) // If a filename is specified, use it. LoadFile(APIFile.c_str()); ExternalNames.insert(APIList.begin(), APIList.end()); + DSONames.insert(DSOList.begin(), DSOList.end()); } -InternalizePass::InternalizePass(ArrayRef ExportList) +InternalizePass::InternalizePass(ArrayRef ExportList, + ArrayRef DSOList) : ModulePass(ID){ initializeInternalizePassPass(*PassRegistry::getPassRegistry()); for(ArrayRef::const_iterator itr = ExportList.begin(); itr != ExportList.end(); itr++) { ExternalNames.insert(*itr); } + for(ArrayRef::const_iterator itr = DSOList.begin(); + itr != DSOList.end(); itr++) { + DSONames.insert(*itr); + } } void InternalizePass::LoadFile(const char *Filename) { @@ -99,7 +112,8 @@ void InternalizePass::LoadFile(const char *Filename) { } static bool shouldInternalize(const GlobalValue &GV, - const std::set &ExternalNames) { + const std::set &ExternalNames, + const std::set &DSONames) { // Function must be defined here if (GV.isDeclaration()) return false; @@ -116,7 +130,20 @@ static bool shouldInternalize(const GlobalValue &GV, if (ExternalNames.count(GV.getName())) return false; - return true; + // Not needed for the symbol table? + if (!DSONames.count(GV.getName())) + return true; + + // Not a linkonce. Someone can depend on it being on the symbol table. + if (!GV.hasLinkOnceLinkage()) + return false; + + // The address is not important, we can hide it. + if (GV.hasUnnamedAddr()) + return true; + + // FIXME: Check if the address is used. + return false; } bool InternalizePass::runOnModule(Module &M) { @@ -145,7 +172,7 @@ bool InternalizePass::runOnModule(Module &M) { // Mark all functions not in the api as internal. // FIXME: maybe use private linkage? for (Module::iterator I = M.begin(), E = M.end(); I != E; ++I) { - if (!shouldInternalize(*I, ExternalNames)) + if (!shouldInternalize(*I, ExternalNames, DSONames)) continue; I->setLinkage(GlobalValue::InternalLinkage); @@ -182,7 +209,7 @@ bool InternalizePass::runOnModule(Module &M) { // FIXME: maybe use private linkage? for (Module::global_iterator I = M.global_begin(), E = M.global_end(); I != E; ++I) { - if (!shouldInternalize(*I, ExternalNames)) + if (!shouldInternalize(*I, ExternalNames, DSONames)) continue; I->setLinkage(GlobalValue::InternalLinkage); @@ -194,7 +221,7 @@ bool InternalizePass::runOnModule(Module &M) { // Mark all aliases that are not in the api as internal as well. for (Module::alias_iterator I = M.alias_begin(), E = M.alias_end(); I != E; ++I) { - if (!shouldInternalize(*I, ExternalNames)) + if (!shouldInternalize(*I, ExternalNames, DSONames)) continue; I->setLinkage(GlobalValue::InternalLinkage); @@ -210,6 +237,7 @@ ModulePass *llvm::createInternalizePass() { return new InternalizePass(); } -ModulePass *llvm::createInternalizePass(ArrayRef ExportList) { - return new InternalizePass(ExportList); +ModulePass *llvm::createInternalizePass(ArrayRef ExportList, + ArrayRef DSOList) { + return new InternalizePass(ExportList, DSOList); } diff --git a/lib/Transforms/IPO/PassManagerBuilder.cpp b/lib/Transforms/IPO/PassManagerBuilder.cpp index 2008c5d65ee..b9660fab78f 100644 --- a/lib/Transforms/IPO/PassManagerBuilder.cpp +++ b/lib/Transforms/IPO/PassManagerBuilder.cpp @@ -277,7 +277,7 @@ void PassManagerBuilder::populateLTOPassManager(PassManagerBase &PM, // for a main function. If main is defined, mark all other functions // internal. if (Internalize) - PM.add(createInternalizePass("main")); + PM.add(createInternalizePass("main", None)); // Propagate constants at call sites into the functions they call. This // opens opportunities for globalopt (and inlining) by substituting function diff --git a/test/LTO/cfi_endproc.ll b/test/LTO/cfi_endproc.ll index 4ea53e7be70..acfaf5a8009 100644 --- a/test/LTO/cfi_endproc.ll +++ b/test/LTO/cfi_endproc.ll @@ -27,3 +27,11 @@ define i32 @main(i32 %argc, i8** %argv) { call void @PR14512() ret i32 0 } + +; RUN: llvm-lto -o %t -dso-symbol=zed1 -dso-symbol=zed2 %t1 -disable-opt +; RUN: llvm-nm %t | FileCheck %s -check-prefix=ZED1_AND_ZED2 +; ZED1_AND_ZED2: V zed1 +@zed1 = linkonce_odr global i32 42 + +; ZED1_AND_ZED2: d zed2 +@zed2 = linkonce_odr unnamed_addr global i32 42 diff --git a/test/Transforms/Internalize/lists.ll b/test/Transforms/Internalize/lists.ll index 83e441a2dfe..59fe0737d3e 100644 --- a/test/Transforms/Internalize/lists.ll +++ b/test/Transforms/Internalize/lists.ll @@ -13,6 +13,10 @@ ; -file and -list options should be merged, the apifile contains foo and j ; RUN: opt < %s -internalize -internalize-public-api-list bar -internalize-public-api-file %S/apifile -S | FileCheck --check-prefix=FOO_J_AND_BAR %s +; Put zed1 and zed2 in the symbol table. If the address is not relevant, we +; internalize them. +; RUN: opt < %s -internalize -internalize-dso-list zed1,zed2 -S | FileCheck --check-prefix=ZED1_AND_ZED2 %s + ; ALL: @i = internal global ; FOO_AND_J: @i = internal global ; FOO_AND_BAR: @i = internal global @@ -25,6 +29,12 @@ ; FOO_J_AND_BAR: @j = global @j = global i32 0 +; ZED1_AND_ZED2: @zed1 = linkonce_odr global i32 42 +@zed1 = linkonce_odr global i32 42 + +; ZED1_AND_ZED2: @zed2 = internal unnamed_addr global i32 42 +@zed2 = linkonce_odr unnamed_addr global i32 42 + ; ALL: define internal void @main() { ; FOO_AND_J: define internal void @main() { ; FOO_AND_BAR: define internal void @main() { diff --git a/tools/gold/gold-plugin.cpp b/tools/gold/gold-plugin.cpp index bd11d1b5556..119631cfa7e 100644 --- a/tools/gold/gold-plugin.cpp +++ b/tools/gold/gold-plugin.cpp @@ -197,7 +197,7 @@ ld_plugin_status onload(ld_plugin_tv *tv) { case LDPT_ADD_SYMBOLS: add_symbols = tv->tv_u.tv_add_symbols; break; - case LDPT_GET_SYMBOLS: + case LDPT_GET_SYMBOLS_V2: get_symbols = tv->tv_u.tv_get_symbols; break; case LDPT_ADD_INPUT_FILE: @@ -386,6 +386,11 @@ static ld_plugin_status all_symbols_read_hook(void) { if (options::generate_api_file) api_file << I->syms[i].name << "\n"; + } else if (I->syms[i].resolution == LDPR_PREVAILING_DEF_IRONLY_EXP) { + lto_codegen_add_dso_symbol(code_gen, I->syms[i].name); + + if (options::generate_api_file) + api_file << I->syms[i].name << " dso only\n"; } } } diff --git a/tools/llvm-lto/llvm-lto.cpp b/tools/llvm-lto/llvm-lto.cpp index 3ecd13f5c71..1d03fa62a87 100644 --- a/tools/llvm-lto/llvm-lto.cpp +++ b/tools/llvm-lto/llvm-lto.cpp @@ -50,6 +50,10 @@ ExportedSymbols("exported-symbol", cl::desc("Symbol to export from the resulting object file"), cl::ZeroOrMore); +static cl::list +DSOSymbols("dso-symbol", + cl::desc("Symbol to put in the symtab in the resulting dso"), + cl::ZeroOrMore); int main(int argc, char **argv) { // Print a stack trace if we signal out. @@ -117,6 +121,10 @@ int main(int argc, char **argv) { for (unsigned i = 0; i < ExportedSymbols.size(); ++i) CodeGen.addMustPreserveSymbol(ExportedSymbols[i].c_str()); + // Add all the dso symbols to the table of symbols to expose. + for (unsigned i = 0; i < DSOSymbols.size(); ++i) + CodeGen.addDSOSymbol(DSOSymbols[i].c_str()); + if (!OutputFilename.empty()) { size_t len = 0; std::string ErrorInfo; diff --git a/tools/lto/lto.cpp b/tools/lto/lto.cpp index 7bfddcd9ec4..a3acd4c9506 100644 --- a/tools/lto/lto.cpp +++ b/tools/lto/lto.cpp @@ -260,6 +260,10 @@ void lto_codegen_add_must_preserve_symbol(lto_code_gen_t cg, cg->addMustPreserveSymbol(symbol); } +void lto_codegen_add_dso_symbol(lto_code_gen_t cg, const char *symbol) { + cg->addDSOSymbol(symbol); +} + /// lto_codegen_write_merged_modules - Writes a new file at the specified path /// that contains the merged contents of all modules added so far. Returns true /// on error (check lto_get_error_message() for details). -- 2.34.1