diff options
author | Pirama Arumuga Nainar <pirama@google.com> | 2018-11-08 20:10:07 +0000 |
---|---|---|
committer | Chih-Hung Hsieh <chh@google.com> | 2018-11-15 11:23:25 -0800 |
commit | 6529da0e68e1d95df0a8200d10bc6c4b0a3a9897 (patch) | |
tree | 3ea88f8e41e1cca5b338ca9908eb4397de4ff427 | |
parent | 05ec7c765159d4ee2471aef366d56a4a5160b5b4 (diff) | |
download | llvm-6529da0e68e1d95df0a8200d10bc6c4b0a3a9897.tar.gz |
[LTO] Drop non-prevailing definitions only if linkage is not local or appending
Summary:
This fixes PR 37422
In ELF, non-weak symbols can also be non-prevailing. In this particular
PR, the __llvm_profile_* symbols are non-prevailing but weren't getting
dropped - causing multiply-defined errors with lld.
Also add a test, strong_non_prevailing.ll, to ensure that multiple
copies of a strong symbol are dropped.
To fix the test regressions exposed by this fix,
- do not mark prevailing copies for symbols with 'appending' linkage.
There's no one prevailing copy for such symbols.
- fix the prevailing version in dead-strip-fulllto.ll
- explicitly pass exported symbols to llvm-lto in fumcimport.ll and
funcimport_var.ll
Reviewers: tejohnson, pcc
Subscribers: mehdi_amini, inglorion, eraman, steven_wu, dexonsmith,
dang, srhines, llvm-commits
Differential Revision: https://reviews.llvm.org/D54125
Change-Id: I25c0597edb56b68b44f49fe2e765893e9643281a
git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@346436 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | include/llvm/LTO/LTO.h | 8 | ||||
-rw-r--r-- | include/llvm/Transforms/IPO/FunctionImport.h | 6 | ||||
-rw-r--r-- | lib/LTO/LTO.cpp | 26 | ||||
-rw-r--r-- | lib/LTO/LTOBackend.cpp | 2 | ||||
-rw-r--r-- | lib/LTO/ThinLTOCodeGenerator.cpp | 20 | ||||
-rw-r--r-- | lib/Transforms/IPO/FunctionImport.cpp | 12 | ||||
-rw-r--r-- | test/LTO/Resolution/X86/dead-strip-fulllto.ll | 8 | ||||
-rw-r--r-- | test/ThinLTO/X86/Inputs/strong_non_prevailing.ll | 6 | ||||
-rw-r--r-- | test/ThinLTO/X86/funcimport.ll | 2 | ||||
-rw-r--r-- | test/ThinLTO/X86/strong_non_prevailing.ll | 16 | ||||
-rw-r--r-- | test/Transforms/FunctionImport/funcimport_var.ll | 2 |
11 files changed, 69 insertions, 39 deletions
diff --git a/include/llvm/LTO/LTO.h b/include/llvm/LTO/LTO.h index 7d6beab6b44..2a3bc9181af 100644 --- a/include/llvm/LTO/LTO.h +++ b/include/llvm/LTO/LTO.h @@ -40,13 +40,13 @@ class Module; class Target; class raw_pwrite_stream; -/// Resolve Weak and LinkOnce values in the \p Index. Linkage changes recorded -/// in the index and the ThinLTO backends must apply the changes to the Module -/// via thinLTOResolveWeakForLinkerModule. +/// Resolve linkage for prevailing symbols in the \p Index. Linkage changes +/// recorded in the index and the ThinLTO backends must apply the changes to +/// the module via thinLTOResolvePrevailingInModule. /// /// This is done for correctness (if value exported, ensure we always /// emit a copy), and compile-time optimization (allow drop of duplicates). -void thinLTOResolveWeakForLinkerInIndex( +void thinLTOResolvePrevailingInIndex( ModuleSummaryIndex &Index, function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)> isPrevailing, diff --git a/include/llvm/Transforms/IPO/FunctionImport.h b/include/llvm/Transforms/IPO/FunctionImport.h index 5ad880574ef..113ef2e4c7b 100644 --- a/include/llvm/Transforms/IPO/FunctionImport.h +++ b/include/llvm/Transforms/IPO/FunctionImport.h @@ -201,10 +201,10 @@ std::error_code EmitImportsFiles( StringRef ModulePath, StringRef OutputFilename, const std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex); -/// Resolve WeakForLinker values in \p TheModule based on the information +/// Resolve prevailing symbol linkages in \p TheModule based on the information /// recorded in the summaries during global summary-based analysis. -void thinLTOResolveWeakForLinkerModule(Module &TheModule, - const GVSummaryMapTy &DefinedGlobals); +void thinLTOResolvePrevailingInModule(Module &TheModule, + const GVSummaryMapTy &DefinedGlobals); /// Internalize \p TheModule based on the information recorded in the summaries /// during global summary-based analysis. diff --git a/lib/LTO/LTO.cpp b/lib/LTO/LTO.cpp index 2726b6785ed..6b11f690ef3 100644 --- a/lib/LTO/LTO.cpp +++ b/lib/LTO/LTO.cpp @@ -282,7 +282,7 @@ static void computeCacheKey( Key = toHex(Hasher.result()); } -static void thinLTOResolveWeakForLinkerGUID( +static void thinLTOResolvePrevailingGUID( GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID, DenseSet<GlobalValueSummary *> &GlobalInvolvedWithAlias, function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)> @@ -291,7 +291,10 @@ static void thinLTOResolveWeakForLinkerGUID( recordNewLinkage) { for (auto &S : GVSummaryList) { GlobalValue::LinkageTypes OriginalLinkage = S->linkage(); - if (!GlobalValue::isWeakForLinker(OriginalLinkage)) + // Ignore local and appending linkage values since the linker + // doesn't resolve them. + if (GlobalValue::isLocalLinkage(OriginalLinkage) || + GlobalValue::isAppendingLinkage(S->linkage())) continue; // We need to emit only one of these. The prevailing module will keep it, // but turned into a weak, while the others will drop it when possible. @@ -315,13 +318,13 @@ static void thinLTOResolveWeakForLinkerGUID( } } -// Resolve Weak and LinkOnce values in the \p Index. +/// Resolve linkage for prevailing symbols in the \p Index. // // We'd like to drop these functions if they are no longer referenced in the // current module. However there is a chance that another module is still // referencing them because of the import. We make sure we always emit at least // one copy. -void llvm::thinLTOResolveWeakForLinkerInIndex( +void llvm::thinLTOResolvePrevailingInIndex( ModuleSummaryIndex &Index, function_ref<bool(GlobalValue::GUID, const GlobalValueSummary *)> isPrevailing, @@ -337,9 +340,9 @@ void llvm::thinLTOResolveWeakForLinkerInIndex( GlobalInvolvedWithAlias.insert(&AS->getAliasee()); for (auto &I : Index) - thinLTOResolveWeakForLinkerGUID(I.second.SummaryList, I.first, - GlobalInvolvedWithAlias, isPrevailing, - recordNewLinkage); + thinLTOResolvePrevailingGUID(I.second.SummaryList, I.first, + GlobalInvolvedWithAlias, isPrevailing, + recordNewLinkage); } static void thinLTOInternalizeAndPromoteGUID( @@ -350,7 +353,10 @@ static void thinLTOInternalizeAndPromoteGUID( if (GlobalValue::isLocalLinkage(S->linkage())) S->setLinkage(GlobalValue::ExternalLinkage); } else if (EnableLTOInternalization && - !GlobalValue::isLocalLinkage(S->linkage())) + // Ignore local and appending linkage values since the linker + // doesn't resolve them. + !GlobalValue::isLocalLinkage(S->linkage()) && + !GlobalValue::isAppendingLinkage(S->linkage())) S->setLinkage(GlobalValue::InternalLinkage); } } @@ -1205,8 +1211,8 @@ Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache) { GlobalValue::LinkageTypes NewLinkage) { ResolvedODR[ModuleIdentifier][GUID] = NewLinkage; }; - thinLTOResolveWeakForLinkerInIndex(ThinLTO.CombinedIndex, isPrevailing, - recordNewLinkage); + thinLTOResolvePrevailingInIndex(ThinLTO.CombinedIndex, isPrevailing, + recordNewLinkage); std::unique_ptr<ThinBackendProc> BackendProc = ThinLTO.Backend(Conf, ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries, diff --git a/lib/LTO/LTOBackend.cpp b/lib/LTO/LTOBackend.cpp index 1f9d60a5bdf..926c419e34a 100644 --- a/lib/LTO/LTOBackend.cpp +++ b/lib/LTO/LTOBackend.cpp @@ -490,7 +490,7 @@ Error lto::thinBackend(Config &Conf, unsigned Task, AddStreamFn AddStream, dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex); - thinLTOResolveWeakForLinkerModule(Mod, DefinedGlobals); + thinLTOResolvePrevailingInModule(Mod, DefinedGlobals); if (Conf.PostPromoteModuleHook && !Conf.PostPromoteModuleHook(Task, Mod)) return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile)); diff --git a/lib/LTO/ThinLTOCodeGenerator.cpp b/lib/LTO/ThinLTOCodeGenerator.cpp index 9500b2ded70..8017527bf22 100644 --- a/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/lib/LTO/ThinLTOCodeGenerator.cpp @@ -457,8 +457,8 @@ ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index, if (!SingleModule) { promoteModule(TheModule, Index); - // Apply summary-based LinkOnce/Weak resolution decisions. - thinLTOResolveWeakForLinkerModule(TheModule, DefinedGlobals); + // Apply summary-based prevailing-symbol resolution decisions. + thinLTOResolvePrevailingInModule(TheModule, DefinedGlobals); // Save temps: after promotion. saveTempBitcode(TheModule, SaveTempsDir, count, ".1.promoted.bc"); @@ -500,12 +500,12 @@ ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index, return codegenModule(TheModule, TM); } -/// Resolve LinkOnce/Weak symbols. Record resolutions in the \p ResolvedODR map +/// Resolve prevailing symbols. Record resolutions in the \p ResolvedODR map /// for caching, and in the \p Index for application during the ThinLTO /// backends. This is needed for correctness for exported symbols (ensure /// at least one copy kept) and a compile-time optimization (to drop duplicate /// copies when possible). -static void resolveWeakForLinkerInIndex( +static void resolvePrevailingInIndex( ModuleSummaryIndex &Index, StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> &ResolvedODR) { @@ -527,7 +527,7 @@ static void resolveWeakForLinkerInIndex( ResolvedODR[ModuleIdentifier][GUID] = NewLinkage; }; - thinLTOResolveWeakForLinkerInIndex(Index, isPrevailing, recordNewLinkage); + thinLTOResolvePrevailingInIndex(Index, isPrevailing, recordNewLinkage); } // Initialize the TargetMachine builder for a given Triple @@ -675,11 +675,11 @@ void ThinLTOCodeGenerator::promote(Module &TheModule, ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists, ExportLists); - // Resolve LinkOnce/Weak symbols. + // Resolve prevailing symbols StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR; - resolveWeakForLinkerInIndex(Index, ResolvedODR); + resolvePrevailingInIndex(Index, ResolvedODR); - thinLTOResolveWeakForLinkerModule( + thinLTOResolvePrevailingInModule( TheModule, ModuleToDefinedGVSummaries[ModuleIdentifier]); // Promote the exported values in the index, so that they are promoted @@ -942,9 +942,9 @@ void ThinLTOCodeGenerator::run() { // on the index, and nuke this map. StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR; - // Resolve LinkOnce/Weak symbols, this has to be computed early because it + // Resolve prevailing symbols, this has to be computed early because it // impacts the caching. - resolveWeakForLinkerInIndex(*Index, ResolvedODR); + resolvePrevailingInIndex(*Index, ResolvedODR); // Use global summary-based analysis to identify symbols that can be // internalized (because they aren't exported or preserved as per callback). diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp index 31531beea5e..1196dd0099b 100644 --- a/lib/Transforms/IPO/FunctionImport.cpp +++ b/lib/Transforms/IPO/FunctionImport.cpp @@ -897,8 +897,8 @@ bool llvm::convertToDeclaration(GlobalValue &GV) { return true; } -/// Fixup WeakForLinker linkages in \p TheModule based on summary analysis. -void llvm::thinLTOResolveWeakForLinkerModule( +/// Fixup prevailing symbol linkages in \p TheModule based on summary analysis. +void llvm::thinLTOResolvePrevailingInModule( Module &TheModule, const GVSummaryMapTy &DefinedGlobals) { auto updateLinkage = [&](GlobalValue &GV) { // See if the global summary analysis computed a new resolved linkage. @@ -915,13 +915,15 @@ void llvm::thinLTOResolveWeakForLinkerModule( // as we need access to the resolution vectors for each input file in // order to find which symbols have been redefined. // We may consider reorganizing this code and moving the linkage recording - // somewhere else, e.g. in thinLTOResolveWeakForLinkerInIndex. + // somewhere else, e.g. in thinLTOResolvePrevailingInIndex. if (NewLinkage == GlobalValue::WeakAnyLinkage) { GV.setLinkage(NewLinkage); return; } - if (!GlobalValue::isWeakForLinker(GV.getLinkage())) + if (GlobalValue::isLocalLinkage(GV.getLinkage()) || + // In case it was dead and already converted to declaration. + GV.isDeclaration()) return; // Check for a non-prevailing def that has interposable linkage // (e.g. non-odr weak or linkonce). In that case we can't simply @@ -932,7 +934,7 @@ void llvm::thinLTOResolveWeakForLinkerModule( GlobalValue::isInterposableLinkage(GV.getLinkage())) { if (!convertToDeclaration(GV)) // FIXME: Change this to collect replaced GVs and later erase - // them from the parent module once thinLTOResolveWeakForLinkerGUID is + // them from the parent module once thinLTOResolvePrevailingGUID is // changed to enable this for aliases. llvm_unreachable("Expected GV to be converted"); } else { diff --git a/test/LTO/Resolution/X86/dead-strip-fulllto.ll b/test/LTO/Resolution/X86/dead-strip-fulllto.ll index 773b4385378..11a8981aceb 100644 --- a/test/LTO/Resolution/X86/dead-strip-fulllto.ll +++ b/test/LTO/Resolution/X86/dead-strip-fulllto.ll @@ -1,14 +1,14 @@ ; RUN: opt -module-summary -o %t %s ; RUN: opt -module-summary -o %t2 %S/Inputs/dead-strip-fulllto.ll -; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1,p -r %t,live2,p -r %t,dead2,p \ -; RUN: %t2 -r %t2,live1, -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, -r %t2,odr, \ +; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1, -r %t,live2,p -r %t,dead2,p \ +; RUN: %t2 -r %t2,live1,p -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, -r %t2,odr, \ ; RUN: -save-temps -o %t3 ; RUN: llvm-nm %t3.0 | FileCheck --check-prefix=FULL %s ; RUN: llvm-nm %t3.1 | FileCheck --check-prefix=THIN %s -; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1,p -r %t,live2,p -r %t,dead2,p \ -; RUN: %t2 -r %t2,live1, -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, -r %t2,odr, \ +; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1, -r %t,live2,p -r %t,dead2,p \ +; RUN: %t2 -r %t2,live1,p -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, -r %t2,odr, \ ; RUN: -save-temps -o %t3 -O0 ; RUN: llvm-nm %t3.0 | FileCheck --check-prefix=FULL %s ; RUN: llvm-nm %t3.1 | FileCheck --check-prefix=THIN %s diff --git a/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll b/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll new file mode 100644 index 00000000000..5473f817f13 --- /dev/null +++ b/test/ThinLTO/X86/Inputs/strong_non_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" + +$__llvm_profile_filename = comdat any + +@__llvm_profile_filename = constant [19 x i8] c"default_%m.profraw\00", comdat diff --git a/test/ThinLTO/X86/funcimport.ll b/test/ThinLTO/X86/funcimport.ll index 1e8784d7cac..fa1bdbf9f11 100644 --- a/test/ThinLTO/X86/funcimport.ll +++ b/test/ThinLTO/X86/funcimport.ll @@ -40,7 +40,7 @@ ; CODEGEN: T _main ; Verify that all run together -; RUN: llvm-lto -thinlto-action=run %t2.bc %t.bc +; RUN: llvm-lto -thinlto-action=run %t2.bc %t.bc -exported-symbol=_main ; RUN: llvm-nm -o - < %t.bc.thinlto.o | FileCheck %s --check-prefix=ALL ; RUN: llvm-nm -o - < %t2.bc.thinlto.o | FileCheck %s --check-prefix=ALL2 ; ALL: T _callfuncptr diff --git a/test/ThinLTO/X86/strong_non_prevailing.ll b/test/ThinLTO/X86/strong_non_prevailing.ll new file mode 100644 index 00000000000..f96e23adc3d --- /dev/null +++ b/test/ThinLTO/X86/strong_non_prevailing.ll @@ -0,0 +1,16 @@ +; RUN: opt -module-summary %s -o %t.bc +; RUN: opt -module-summary %p/Inputs/strong_non_prevailing.ll -o %t2.bc + +; RUN: llvm-lto -thinlto-action=run %t.bc %t2.bc -exported-symbol=__llvm_profile_filename +; RUN: llvm-nm -o - < %t.bc.thinlto.o | FileCheck %s --check-prefix=EXPORTED +; RUN: llvm-nm -o - < %t2.bc.thinlto.o 2>&1 | FileCheck %s --check-prefix=NOT_EXPORTED + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +$__llvm_profile_filename = comdat any + +@__llvm_profile_filename = constant [19 x i8] c"default_%m.profraw\00", comdat + +; EXPORTED: N __llvm_profile_filename +; NOT_EXPORTED-NOT: N __llvm_profile_filename diff --git a/test/Transforms/FunctionImport/funcimport_var.ll b/test/Transforms/FunctionImport/funcimport_var.ll index a93cabba69a..edd874e6297 100644 --- a/test/Transforms/FunctionImport/funcimport_var.ll +++ b/test/Transforms/FunctionImport/funcimport_var.ll @@ -4,7 +4,7 @@ ; RUN: opt -module-summary %p/Inputs/funcimport_var2.ll -o %t2.bc ; RUN: llvm-lto -thinlto -thinlto-action=thinlink -o %t3 %t.bc %t2.bc ; RUN: llvm-lto -thinlto -thinlto-action=import -thinlto-index=%t3 %t.bc %t2.bc -; RUN: llvm-lto -thinlto -thinlto-action=run %t.bc %t2.bc +; RUN: llvm-lto -thinlto -thinlto-action=run %t.bc %t2.bc -exported-symbol=_Z4LinkPKcS0_ ; RUN: llvm-nm %t.bc.thinlto.o | FileCheck %s ; RUN: llvm-lto2 run %t.bc %t2.bc -o %t.out \ ; RUN: -r %t.bc,_Z4LinkPKcS0_,plx \ |