From bd573457622a5f909a1ebc4b6ba3b5600f25674d Mon Sep 17 00:00:00 2001 From: Mehdi Amini Date: Fri, 3 Feb 2017 00:32:38 +0000 Subject: [PATCH] [ThinLTO] Add an auto-hide feature When a symbol is not exported outside of the DSO, it is can be hidden. Usually we try to internalize as much as possible, but it is not always possible, for instance a symbol can be referenced outside of the LTO unit, or there can be cross-module reference in ThinLTO. This is a recommit of r293912 after fixing build failures, and a recommit of r293918 after fixing LLD tests. Differential Revision: https://reviews.llvm.org/D28978 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@293970 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/ModuleSummaryIndex.h | 10 ++++-- include/llvm/IR/ModuleSummaryIndexYAML.h | 2 +- include/llvm/LTO/LTO.h | 8 ++++- lib/Analysis/ModuleSummaryAnalysis.cpp | 12 ++++--- lib/Bitcode/Reader/BitcodeReader.cpp | 9 ++--- lib/Bitcode/Writer/BitcodeWriter.cpp | 8 ++--- lib/LTO/LTO.cpp | 26 ++++++++++---- lib/LTO/ThinLTOCodeGenerator.cpp | 60 ++++++++++++++++++-------------- lib/Transforms/IPO/FunctionImport.cpp | 7 ++++ test/ThinLTO/X86/Inputs/weak_autohide.ll | 6 ++++ test/ThinLTO/X86/deadstrip.ll | 7 ++-- test/ThinLTO/X86/weak_autohide.ll | 24 +++++++++++++ 12 files changed, 127 insertions(+), 52 deletions(-) create mode 100644 test/ThinLTO/X86/Inputs/weak_autohide.ll create mode 100644 test/ThinLTO/X86/weak_autohide.ll diff --git a/include/llvm/IR/ModuleSummaryIndex.h b/include/llvm/IR/ModuleSummaryIndex.h index c710c41cccd..967b5c7a859 100644 --- a/include/llvm/IR/ModuleSummaryIndex.h +++ b/include/llvm/IR/ModuleSummaryIndex.h @@ -126,11 +126,14 @@ public: /// llvm.global_ctors that the linker does not know about. unsigned LiveRoot : 1; + /// Indicate if the global value should be hidden. + unsigned AutoHide : 1; + /// Convenience Constructors explicit GVFlags(GlobalValue::LinkageTypes Linkage, - bool NotEligibleToImport, bool LiveRoot) + bool NotEligibleToImport, bool LiveRoot, bool AutoHide) : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport), - LiveRoot(LiveRoot) {} + LiveRoot(LiveRoot), AutoHide(AutoHide) {} }; private: @@ -198,6 +201,9 @@ public: Flags.Linkage = Linkage; } + /// Sets the visibility to be autohidden. + void setAutoHide() { Flags.AutoHide = true; } + /// Return true if this global value can't be imported. bool notEligibleToImport() const { return Flags.NotEligibleToImport; } diff --git a/include/llvm/IR/ModuleSummaryIndexYAML.h b/include/llvm/IR/ModuleSummaryIndexYAML.h index e2880ec6fec..c880abfea24 100644 --- a/include/llvm/IR/ModuleSummaryIndexYAML.h +++ b/include/llvm/IR/ModuleSummaryIndexYAML.h @@ -79,7 +79,7 @@ template <> struct CustomMappingTraits { auto &Elem = V[KeyInt]; for (auto &FSum : FSums) { GlobalValueSummary::GVFlags GVFlags(GlobalValue::ExternalLinkage, false, - false); + false, /* AutoHide */ false); Elem.push_back(llvm::make_unique( GVFlags, 0, ArrayRef{}, ArrayRef{}, std::move(FSum.TypeTests))); diff --git a/include/llvm/LTO/LTO.h b/include/llvm/LTO/LTO.h index 2aeb7e8fda9..c346ec522df 100644 --- a/include/llvm/LTO/LTO.h +++ b/include/llvm/LTO/LTO.h @@ -53,12 +53,18 @@ void thinLTOResolveWeakForLinkerInIndex( function_ref recordNewLinkage); +/// This enum is used for the returned value of the callback passed to +/// thinLTOInternalizeAndPromoteInIndex, it indicates if a symbol can be made +/// Internal (only referenced from its defining object), Hidden ( +/// outside the DSO), or Exported (exposed as public API for the DSO). +enum SummaryResolution { Internal, Hidden, Exported }; + /// Update the linkages in the given \p Index to mark exported values /// as external and non-exported values as internal. The ThinLTO backends /// must apply the changes to the Module via thinLTOInternalizeModule. void thinLTOInternalizeAndPromoteInIndex( ModuleSummaryIndex &Index, - function_ref isExported); + function_ref isExported); namespace lto { diff --git a/lib/Analysis/ModuleSummaryAnalysis.cpp b/lib/Analysis/ModuleSummaryAnalysis.cpp index f5ba637e58e..0227eab0ede 100644 --- a/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -190,7 +190,8 @@ computeFunctionSummary(ModuleSummaryIndex &Index, const Module &M, // FIXME: refactor this to use the same code that inliner is using. F.isVarArg(); GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport, - /* LiveRoot = */ false); + /* LiveRoot = */ false, + /* AutoHide */ false); auto FuncSummary = llvm::make_unique( Flags, NumInsts, RefEdges.takeVector(), CallGraphEdges.takeVector(), TypeTests.takeVector()); @@ -207,7 +208,8 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V, findRefEdges(&V, RefEdges, Visited); bool NonRenamableLocal = isNonRenamableLocal(V); GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal, - /* LiveRoot = */ false); + /* LiveRoot = */ false, + /* AutoHide */ false); auto GVarSummary = llvm::make_unique(Flags, RefEdges.takeVector()); if (NonRenamableLocal) @@ -220,7 +222,8 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A, DenseSet &CantBePromoted) { bool NonRenamableLocal = isNonRenamableLocal(A); GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal, - /* LiveRoot = */ false); + /* LiveRoot = */ false, + /* AutoHide */ false); auto AS = llvm::make_unique(Flags, ArrayRef{}); auto *Aliasee = A.getBaseObject(); auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee); @@ -339,7 +342,8 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( assert(GV->isDeclaration() && "Def in module asm already has definition"); GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage, /* NotEligibleToImport */ true, - /* LiveRoot */ true); + /* LiveRoot */ true, + /* AutoHide */ false); CantBePromoted.insert(GlobalValue::getGUID(Name)); // Create the appropriate summary type. if (isa(GV)) { diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index a46e49ccde8..c9d2ad5c71b 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -800,13 +800,14 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags, // like getDecodedLinkage() above. Any future change to the linkage enum and // to getDecodedLinkage() will need to be taken into account here as above. auto Linkage = GlobalValue::LinkageTypes(RawFlags & 0xF); // 4 bits - RawFlags = RawFlags >> 4; - bool NotEligibleToImport = (RawFlags & 0x1) || Version < 3; + bool NotEligibleToImport = ((RawFlags >> 4) & 0x1) || Version < 3; // The LiveRoot flag wasn't introduced until version 3. For dead stripping // to work correctly on earlier versions, we must conservatively treat all // values as live. - bool LiveRoot = (RawFlags & 0x2) || Version < 3; - return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, LiveRoot); + bool LiveRoot = ((RawFlags >> 5) & 0x1) || Version < 3; + bool AutoHide = (RawFlags >> 6) & 0x1; + return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, LiveRoot, + AutoHide); } static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) { diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index 4eac89c37a5..bdb57f59dde 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -971,13 +971,13 @@ static unsigned getEncodedLinkage(const GlobalValue &GV) { static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) { uint64_t RawFlags = 0; - RawFlags |= Flags.NotEligibleToImport; // bool - RawFlags |= (Flags.LiveRoot << 1); // Linkage don't need to be remapped at that time for the summary. Any future // change to the getEncodedLinkage() function will need to be taken into // account here as well. - RawFlags = (RawFlags << 4) | Flags.Linkage; // 4 bits - + RawFlags |= Flags.Linkage; // 4 bits linkage + RawFlags |= (Flags.NotEligibleToImport << 4); // bool + RawFlags |= (Flags.LiveRoot << 5); // bool + RawFlags |= (Flags.AutoHide << 6); // bool return RawFlags; } diff --git a/lib/LTO/LTO.cpp b/lib/LTO/LTO.cpp index bf30fec5d20..db0253161b8 100644 --- a/lib/LTO/LTO.cpp +++ b/lib/LTO/LTO.cpp @@ -203,11 +203,14 @@ void llvm::thinLTOResolveWeakForLinkerInIndex( static void thinLTOInternalizeAndPromoteGUID( GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID, - function_ref isExported) { + function_ref isExported) { for (auto &S : GVSummaryList) { - if (isExported(S->modulePath(), GUID)) { + auto ExportResolution = isExported(S->modulePath(), GUID); + if (ExportResolution != Internal) { if (GlobalValue::isLocalLinkage(S->linkage())) S->setLinkage(GlobalValue::ExternalLinkage); + if (ExportResolution == Hidden) + S->setAutoHide(); } else if (!GlobalValue::isLocalLinkage(S->linkage())) S->setLinkage(GlobalValue::InternalLinkage); } @@ -217,7 +220,7 @@ static void thinLTOInternalizeAndPromoteGUID( // as external and non-exported values as internal. void llvm::thinLTOInternalizeAndPromoteInIndex( ModuleSummaryIndex &Index, - function_ref isExported) { + function_ref isExported) { for (auto &I : Index) thinLTOInternalizeAndPromoteGUID(I.second, I.first, isExported); } @@ -951,11 +954,20 @@ Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache, const GlobalValueSummary *S) { return ThinLTO.PrevailingModuleForGUID[GUID] == S->modulePath(); }; - auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) { + auto isExported = [&](StringRef ModuleIdentifier, + GlobalValue::GUID GUID) -> SummaryResolution { const auto &ExportList = ExportLists.find(ModuleIdentifier); - return (ExportList != ExportLists.end() && - ExportList->second.count(GUID)) || - ExportedGUIDs.count(GUID); + if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) || + ExportedGUIDs.count(GUID)) { + // We could do better by hiding when a symbol is in + // GUIDPreservedSymbols because it is only referenced from regular LTO + // or from native files and not outside the final binary, but that's + // something the native linker could do as gwell. + if (GUIDPreservedSymbols.count(GUID)) + return Exported; + return Hidden; + } + return Internal; }; thinLTOInternalizeAndPromoteInIndex(ThinLTO.CombinedIndex, isExported); diff --git a/lib/LTO/ThinLTOCodeGenerator.cpp b/lib/LTO/ThinLTOCodeGenerator.cpp index 104fb199da0..b62db61f4c4 100644 --- a/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/lib/LTO/ThinLTOCodeGenerator.cpp @@ -234,16 +234,16 @@ static void optimizeModule(Module &TheModule, TargetMachine &TM, // Convert the PreservedSymbols map from "Name" based to "GUID" based. static DenseSet -computeGUIDPreservedSymbols(const StringSet<> &PreservedSymbols, - const Triple &TheTriple) { - DenseSet GUIDPreservedSymbols(PreservedSymbols.size()); - for (auto &Entry : PreservedSymbols) { +convertSymbolNamesToGUID(const StringSet<> &NamedSymbols, + const Triple &TheTriple) { + DenseSet GUIDSymbols(NamedSymbols.size()); + for (auto &Entry : NamedSymbols) { StringRef Name = Entry.first(); if (TheTriple.isOSBinFormatMachO() && Name.size() > 0 && Name[0] == '_') Name = Name.drop_front(); - GUIDPreservedSymbols.insert(GlobalValue::getGUID(Name)); + GUIDSymbols.insert(GlobalValue::getGUID(Name)); } - return GUIDPreservedSymbols; + return GUIDSymbols; } std::unique_ptr codegenModule(Module &TheModule, @@ -554,10 +554,7 @@ void ThinLTOCodeGenerator::preserveSymbol(StringRef Name) { } void ThinLTOCodeGenerator::crossReferenceSymbol(StringRef Name) { - // FIXME: At the moment, we don't take advantage of this extra information, - // we're conservatively considering cross-references as preserved. - // CrossReferencedSymbols.insert(Name); - PreservedSymbols.insert(Name); + CrossReferencedSymbols.insert(Name); } // TargetMachine factory @@ -620,7 +617,7 @@ void ThinLTOCodeGenerator::promote(Module &TheModule, Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries); // Convert the preserved symbols set from string to GUID - auto GUIDPreservedSymbols = computeGUIDPreservedSymbols( + auto GUIDPreservedSymbols = convertSymbolNamesToGUID( PreservedSymbols, Triple(TheModule.getTargetTriple())); // Compute "dead" symbols, we don't want to import/export these! @@ -641,11 +638,13 @@ void ThinLTOCodeGenerator::promote(Module &TheModule, // Promote the exported values in the index, so that they are promoted // in the module. - auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) { + auto isExported = [&](StringRef ModuleIdentifier, + GlobalValue::GUID GUID) -> SummaryResolution { const auto &ExportList = ExportLists.find(ModuleIdentifier); - return (ExportList != ExportLists.end() && - ExportList->second.count(GUID)) || - GUIDPreservedSymbols.count(GUID); + if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) || + GUIDPreservedSymbols.count(GUID)) + return Exported; + return Internal; }; thinLTOInternalizeAndPromoteInIndex(Index, isExported); @@ -665,7 +664,7 @@ void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule, Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries); // Convert the preserved symbols set from string to GUID - auto GUIDPreservedSymbols = computeGUIDPreservedSymbols( + auto GUIDPreservedSymbols = convertSymbolNamesToGUID( PreservedSymbols, Triple(TheModule.getTargetTriple())); // Compute "dead" symbols, we don't want to import/export these! @@ -739,7 +738,7 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule, // Convert the preserved symbols set from string to GUID auto GUIDPreservedSymbols = - computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple); + convertSymbolNamesToGUID(PreservedSymbols, TMBuilder.TheTriple); // Collect for each module the list of function it defines (GUID -> Summary). StringMap ModuleToDefinedGVSummaries(ModuleCount); @@ -761,11 +760,13 @@ void ThinLTOCodeGenerator::internalize(Module &TheModule, return; // Internalization - auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) { + auto isExported = [&](StringRef ModuleIdentifier, + GlobalValue::GUID GUID) -> SummaryResolution { const auto &ExportList = ExportLists.find(ModuleIdentifier); - return (ExportList != ExportLists.end() && - ExportList->second.count(GUID)) || - GUIDPreservedSymbols.count(GUID); + if ((ExportList != ExportLists.end() && ExportList->second.count(GUID)) || + GUIDPreservedSymbols.count(GUID)) + return Exported; + return Internal; }; thinLTOInternalizeAndPromoteInIndex(Index, isExported); thinLTOInternalizeModule(TheModule, @@ -894,7 +895,9 @@ void ThinLTOCodeGenerator::run() { // Convert the preserved symbols set from string to GUID, this is needed for // computing the caching hash and the internalization. auto GUIDPreservedSymbols = - computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple); + convertSymbolNamesToGUID(PreservedSymbols, TMBuilder.TheTriple); + auto GUIDCrossRefSymbols = + convertSymbolNamesToGUID(CrossReferencedSymbols, TMBuilder.TheTriple); // Compute "dead" symbols, we don't want to import/export these! auto DeadSymbols = computeDeadSymbols(*Index, GUIDPreservedSymbols); @@ -916,11 +919,16 @@ void ThinLTOCodeGenerator::run() { // impacts the caching. resolveWeakForLinkerInIndex(*Index, ResolvedODR); - auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) { + auto isExported = [&](StringRef ModuleIdentifier, + GlobalValue::GUID GUID) -> SummaryResolution { + if (GUIDPreservedSymbols.count(GUID)) + return Exported; + if (GUIDCrossRefSymbols.count(GUID)) + return Hidden; const auto &ExportList = ExportLists.find(ModuleIdentifier); - return (ExportList != ExportLists.end() && - ExportList->second.count(GUID)) || - GUIDPreservedSymbols.count(GUID); + if (ExportList != ExportLists.end() && ExportList->second.count(GUID)) + return Hidden; + return Internal; }; // Use global summary-based analysis to identify symbols that can be diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp index 5fa99863135..2966be2d46d 100644 --- a/lib/Transforms/IPO/FunctionImport.cpp +++ b/lib/Transforms/IPO/FunctionImport.cpp @@ -644,6 +644,13 @@ void llvm::thinLTOInternalizeModule(Module &TheModule, return !GlobalValue::isLocalLinkage(Linkage); }; + // Try to auto-hide the symbols. + for (auto &GO : TheModule.global_objects()) { + const auto &GS = DefinedGlobals.find(GO.getGUID()); + if (GS != DefinedGlobals.end() && GS->second->flags().AutoHide) + GO.setVisibility(GlobalValue::HiddenVisibility); + } + // FIXME: See if we can just internalize directly here via linkage changes // based on the index, rather than invoking internalizeModule. llvm::internalizeModule(TheModule, MustPreserveGV); diff --git a/test/ThinLTO/X86/Inputs/weak_autohide.ll b/test/ThinLTO/X86/Inputs/weak_autohide.ll new file mode 100644 index 00000000000..c9c2bfd01ac --- /dev/null +++ b/test/ThinLTO/X86/Inputs/weak_autohide.ll @@ -0,0 +1,6 @@ +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.11.0" + +define weak_odr void @weakodrfunc() { + ret void +} diff --git a/test/ThinLTO/X86/deadstrip.ll b/test/ThinLTO/X86/deadstrip.ll index 6f1cbfe5969..14e7bae6cd9 100644 --- a/test/ThinLTO/X86/deadstrip.ll +++ b/test/ThinLTO/X86/deadstrip.ll @@ -3,7 +3,7 @@ ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t1.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t1.bc - -o - | llvm-dis -o - | FileCheck %s -; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t2.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK2 +; RUN: llvm-lto -exported-symbol=_main -thinlto-action=promote %t2.bc -thinlto-index=%t.index.bc -o - | llvm-lto -exported-symbol=_main -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=CHECK2-LTO ; RUN: llvm-lto -exported-symbol=_main -thinlto-action=run %t1.bc %t2.bc ; RUN: llvm-nm %t1.bc.thinlto.o | FileCheck %s --check-prefix=CHECK-NM @@ -19,7 +19,7 @@ ; RUN: -r %t2.bc,_dead_func,pl \ ; RUN: -r %t2.bc,_another_dead_func,pl ; RUN: llvm-dis < %t.out.0.3.import.bc | FileCheck %s -; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=CHECK2 +; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=CHECK2-LTO2 ; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM ; Dead-stripping on the index allows to internalize these, @@ -34,7 +34,8 @@ ; Make sure we didn't internalize @boo, which is reachable via ; llvm.global_ctors -; CHECK2: define void @boo() +; CHECK2-LTO: define void @boo() +; CHECK2-LTO2: define hidden void @boo() ; We should have eventually revoved @baz since it was internalized and unused ; CHECK2-NM-NOT: _baz diff --git a/test/ThinLTO/X86/weak_autohide.ll b/test/ThinLTO/X86/weak_autohide.ll new file mode 100644 index 00000000000..063e36db51c --- /dev/null +++ b/test/ThinLTO/X86/weak_autohide.ll @@ -0,0 +1,24 @@ +; RUN: opt -module-summary %s -o %t1.bc +; RUN: opt -module-summary %p/Inputs/weak_autohide.ll -o %t2.bc + +; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \ +; RUN: -r=%t1.bc,_strong_func,pxl \ +; RUN: -r=%t1.bc,_weakodrfunc,pl \ +; RUN: -r=%t2.bc,_weakodrfunc,l +; RUN: llvm-dis < %t.o.0.2.internalize.bc | FileCheck %s --check-prefix=AUTOHIDE + + +; AUTOHIDE: weak_odr hidden void @weakodrfunc + +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.11.0" + +define weak_odr void @weakodrfunc() #0 { + ret void +} + +define void @strong_func() #0 { + call void @weakodrfunc() + ret void +} + -- 2.11.0