[llvm] r346436 - [LTO] Drop non-prevailing definitions only if linkage is not local or appending
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 3 04:29:00 PST 2018
For those following along, this broke Chromium's CFI build, see
https://bugs.chromium.org/p/chromium/issues/detail?id=910644
On Thu, Nov 8, 2018 at 9:12 PM Pirama Arumuga Nainar via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: pirama
> Date: Thu Nov 8 12:10:07 2018
> New Revision: 346436
>
> URL: http://llvm.org/viewvc/llvm-project?rev=346436&view=rev
> Log:
> [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
>
> Added:
> llvm/trunk/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll
> llvm/trunk/test/ThinLTO/X86/strong_non_prevailing.ll
> Modified:
> llvm/trunk/include/llvm/LTO/LTO.h
> llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h
> llvm/trunk/lib/LTO/LTO.cpp
> llvm/trunk/lib/LTO/LTOBackend.cpp
> llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
> llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> llvm/trunk/test/LTO/Resolution/X86/dead-strip-fulllto.ll
> llvm/trunk/test/ThinLTO/X86/funcimport.ll
> llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll
>
> Modified: llvm/trunk/include/llvm/LTO/LTO.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/LTO/LTO.h?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/LTO/LTO.h (original)
> +++ llvm/trunk/include/llvm/LTO/LTO.h Thu Nov 8 12:10:07 2018
> @@ -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,
>
> Modified: llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h (original)
> +++ llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h Thu Nov 8 12:10:07 2018
> @@ -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.
>
> Modified: llvm/trunk/lib/LTO/LTO.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/LTO.cpp (original)
> +++ llvm/trunk/lib/LTO/LTO.cpp Thu Nov 8 12:10:07 2018
> @@ -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 thinLTOResolveWeakForLinkerG
> 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 thinLTOResolveWeakForLinkerG
> }
> }
>
> -// 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::thinLTOResolveWeakForLinkerIn
> 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 thinLTOInternalizeAndPromote
> 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 AddStr
> 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,
>
> Modified: llvm/trunk/lib/LTO/LTOBackend.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOBackend.cpp?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/LTOBackend.cpp (original)
> +++ llvm/trunk/lib/LTO/LTOBackend.cpp Thu Nov 8 12:10:07 2018
> @@ -490,7 +490,7 @@ Error lto::thinBackend(Config &Conf, uns
>
> dropDeadSymbols(Mod, DefinedGlobals, CombinedIndex);
>
> - thinLTOResolveWeakForLinkerModule(Mod, DefinedGlobals);
> + thinLTOResolvePrevailingInModule(Mod, DefinedGlobals);
>
> if (Conf.PostPromoteModuleHook && !Conf.PostPromoteModuleHook(Task, Mod))
> return finalizeOptimizationRemarks(std::move(DiagnosticOutputFile));
>
> Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
> +++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Thu Nov 8 12:10:07 2018
> @@ -457,8 +457,8 @@ ProcessThinLTOModule(Module &TheModule,
> 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,
> 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(Modul
> 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).
>
> Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Thu Nov 8 12:10:07 2018
> @@ -897,8 +897,8 @@ bool llvm::convertToDeclaration(GlobalVa
> 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::thinLTOResolveWeakForLinkerMo
> // 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::thinLTOResolveWeakForLinkerMo
> 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 {
>
> Modified: llvm/trunk/test/LTO/Resolution/X86/dead-strip-fulllto.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/dead-strip-fulllto.ll?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/test/LTO/Resolution/X86/dead-strip-fulllto.ll (original)
> +++ llvm/trunk/test/LTO/Resolution/X86/dead-strip-fulllto.ll Thu Nov 8 12:10:07 2018
> @@ -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
>
> Added: llvm/trunk/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll?rev=346436&view=auto
> ==============================================================================
> --- llvm/trunk/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll (added)
> +++ llvm/trunk/test/ThinLTO/X86/Inputs/strong_non_prevailing.ll Thu Nov 8 12:10:07 2018
> @@ -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
> +
> + at __llvm_profile_filename = constant [19 x i8] c"default_%m.profraw\00", comdat
>
> Modified: llvm/trunk/test/ThinLTO/X86/funcimport.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/funcimport.ll?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/test/ThinLTO/X86/funcimport.ll (original)
> +++ llvm/trunk/test/ThinLTO/X86/funcimport.ll Thu Nov 8 12:10:07 2018
> @@ -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
>
> Added: llvm/trunk/test/ThinLTO/X86/strong_non_prevailing.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/strong_non_prevailing.ll?rev=346436&view=auto
> ==============================================================================
> --- llvm/trunk/test/ThinLTO/X86/strong_non_prevailing.ll (added)
> +++ llvm/trunk/test/ThinLTO/X86/strong_non_prevailing.ll Thu Nov 8 12:10:07 2018
> @@ -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
> +
> + at __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
>
> Modified: llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll?rev=346436&r1=346435&r2=346436&view=diff
> ==============================================================================
> --- llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll (original)
> +++ llvm/trunk/test/Transforms/FunctionImport/funcimport_var.ll Thu Nov 8 12:10:07 2018
> @@ -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 \
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list