From 4595a915f6d1e782e3e297eb611cbcac46af9bcb Mon Sep 17 00:00:00 2001 From: Sean Fertile Date: Sat, 4 Nov 2017 17:04:39 +0000 Subject: [PATCH] [LTO][ThinLTO] Use the linker resolutions to mark global values as dso_local. Now that we have a way to mark GlobalValues as local we can use the symbol resolutions that the linker plugin provides as part of lto/thinlto link step to refine the compilers view on what symbols will end up being local. Originally commited as r317374, but reverted in r317395 to update some missed tests. Differential Revision: https://reviews.llvm.org/D35702 llvm-svn: 317408 --- llvm/include/llvm/IR/ModuleSummaryIndex.h | 12 +++++++-- llvm/include/llvm/IR/ModuleSummaryIndexYAML.h | 8 +++--- llvm/lib/Analysis/ModuleSummaryAnalysis.cpp | 9 ++++--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 4 ++- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 2 ++ llvm/lib/LTO/LTO.cpp | 21 +++++++++++---- llvm/lib/Transforms/Utils/FunctionImportUtils.cpp | 17 ++++++++++++ llvm/test/Bitcode/thinlto-summary-local-5.0.ll | 22 +++++++++++++++ llvm/test/Bitcode/thinlto-summary-local-5.0.ll.bc | Bin 0 -> 1028 bytes llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll | 2 +- llvm/test/LTO/Resolution/X86/comdat.ll | 4 +-- llvm/test/LTO/Resolution/X86/commons.ll | 2 +- llvm/test/ThinLTO/X86/deadstrip.ll | 30 +++++++++++++-------- llvm/test/ThinLTO/X86/funcimport2.ll | 4 +-- llvm/test/ThinLTO/X86/internalize.ll | 9 ++++--- llvm/test/ThinLTO/X86/reference_non_importable.ll | 2 +- .../test/Transforms/LowerTypeTests/import-unsat.ll | 1 + .../PGOProfile/thinlto_samplepgo_icp2.ll | 2 +- .../Transforms/WholeProgramDevirt/import-indir.ll | 1 + llvm/test/tools/gold/X86/asm_undefined2.ll | 3 ++- llvm/test/tools/gold/X86/coff.ll | 2 +- llvm/test/tools/gold/X86/common.ll | 2 +- llvm/test/tools/gold/X86/emit-llvm.ll | 6 ++--- llvm/test/tools/gold/X86/global_with_section.ll | 16 +++++------ llvm/test/tools/gold/X86/parallel.ll | 8 +++--- .../tools/gold/X86/thinlto_linkonceresolution.ll | 2 +- llvm/test/tools/gold/X86/thinlto_weak_library.ll | 2 +- llvm/test/tools/gold/X86/visibility.ll | 2 +- 28 files changed, 137 insertions(+), 58 deletions(-) create mode 100644 llvm/test/Bitcode/thinlto-summary-local-5.0.ll create mode 100644 llvm/test/Bitcode/thinlto-summary-local-5.0.ll.bc diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h index 2d664f4..b1e58a2 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -148,11 +148,15 @@ public: /// In combined summary, indicate that the global value is live. unsigned Live : 1; + /// Indicates that the linker resolved the symbol to a definition from + /// within the same linkage unit. + unsigned DSOLocal : 1; + /// Convenience Constructors explicit GVFlags(GlobalValue::LinkageTypes Linkage, - bool NotEligibleToImport, bool Live) + bool NotEligibleToImport, bool Live, bool IsLocal) : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport), - Live(Live) {} + Live(Live), DSOLocal(IsLocal) {} }; private: @@ -229,6 +233,10 @@ public: void setLive(bool Live) { Flags.Live = Live; } + void setDSOLocal(bool Local) { Flags.DSOLocal = Local; } + + bool isDSOLocal() const { return Flags.DSOLocal; } + /// Flag that this global value cannot be imported. void setNotEligibleToImport() { Flags.NotEligibleToImport = true; } diff --git a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h index 2f9990c..4687f2d 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndexYAML.h @@ -135,7 +135,7 @@ template <> struct MappingTraits { struct FunctionSummaryYaml { unsigned Linkage; - bool NotEligibleToImport, Live; + bool NotEligibleToImport, Live, IsLocal; std::vector TypeTests; std::vector TypeTestAssumeVCalls, TypeCheckedLoadVCalls; @@ -177,6 +177,7 @@ template <> struct MappingTraits { io.mapOptional("Linkage", summary.Linkage); io.mapOptional("NotEligibleToImport", summary.NotEligibleToImport); io.mapOptional("Live", summary.Live); + io.mapOptional("Local", summary.IsLocal); io.mapOptional("TypeTests", summary.TypeTests); io.mapOptional("TypeTestAssumeVCalls", summary.TypeTestAssumeVCalls); io.mapOptional("TypeCheckedLoadVCalls", summary.TypeCheckedLoadVCalls); @@ -211,7 +212,7 @@ template <> struct CustomMappingTraits { Elem.SummaryList.push_back(llvm::make_unique( GlobalValueSummary::GVFlags( static_cast(FSum.Linkage), - FSum.NotEligibleToImport, FSum.Live), + FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal), 0, FunctionSummary::FFlags{}, ArrayRef{}, ArrayRef{}, std::move(FSum.TypeTests), std::move(FSum.TypeTestAssumeVCalls), @@ -228,7 +229,8 @@ template <> struct CustomMappingTraits { FSums.push_back(FunctionSummaryYaml{ FSum->flags().Linkage, static_cast(FSum->flags().NotEligibleToImport), - static_cast(FSum->flags().Live), FSum->type_tests(), + static_cast(FSum->flags().Live), + static_cast(FSum->flags().DSOLocal), FSum->type_tests(), FSum->type_test_assume_vcalls(), FSum->type_checked_load_vcalls(), FSum->type_test_assume_const_vcalls(), FSum->type_checked_load_const_vcalls()}); diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp index afd575e..82db09c 100644 --- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp +++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp @@ -303,7 +303,7 @@ 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, - /* Live = */ false); + /* Live = */ false, F.isDSOLocal()); FunctionSummary::FFlags FunFlags{ F.hasFnAttribute(Attribute::ReadNone), F.hasFnAttribute(Attribute::ReadOnly), @@ -329,7 +329,7 @@ computeVariableSummary(ModuleSummaryIndex &Index, const GlobalVariable &V, findRefEdges(Index, &V, RefEdges, Visited); bool NonRenamableLocal = isNonRenamableLocal(V); GlobalValueSummary::GVFlags Flags(V.getLinkage(), NonRenamableLocal, - /* Live = */ false); + /* Live = */ false, V.isDSOLocal()); auto GVarSummary = llvm::make_unique(Flags, RefEdges.takeVector()); if (NonRenamableLocal) @@ -342,7 +342,7 @@ computeAliasSummary(ModuleSummaryIndex &Index, const GlobalAlias &A, DenseSet &CantBePromoted) { bool NonRenamableLocal = isNonRenamableLocal(A); GlobalValueSummary::GVFlags Flags(A.getLinkage(), NonRenamableLocal, - /* Live = */ false); + /* Live = */ false, A.isDSOLocal()); auto AS = llvm::make_unique(Flags); auto *Aliasee = A.getBaseObject(); auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee); @@ -410,7 +410,8 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( assert(GV->isDeclaration() && "Def in module asm already has definition"); GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage, /* NotEligibleToImport = */ true, - /* Live = */ true); + /* Live = */ true, + /* Local */ GV->isDSOLocal()); CantBePromoted.insert(GlobalValue::getGUID(Name)); // Create the appropriate summary type. if (Function *F = dyn_cast(GV)) { diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index c227226..d0f11db 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -889,7 +889,9 @@ static GlobalValueSummary::GVFlags getDecodedGVSummaryFlags(uint64_t RawFlags, // to work correctly on earlier versions, we must conservatively treat all // values as live. bool Live = (RawFlags & 0x2) || Version < 3; - return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live); + bool Local = (RawFlags & 0x4); + + return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live, Local); } static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) { diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index 1e491aa..c5d376c 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -955,6 +955,8 @@ static uint64_t getEncodedGVSummaryFlags(GlobalValueSummary::GVFlags Flags) { RawFlags |= Flags.NotEligibleToImport; // bool RawFlags |= (Flags.Live << 1); + RawFlags |= (Flags.DSOLocal << 2); + // 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. diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 017dd20..9c73779 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -630,6 +630,9 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef Syms, NonPrevailingComdats.insert(GV->getComdat()); cast(GV)->setComdat(nullptr); } + + // Set the 'local' flag based on the linker resolution for this symbol. + GV->setDSOLocal(Res.FinalDefinitionInLinkageUnit); } // Common resolution: collect the maximum size/alignment over all commons. // We also record if we see an instance of a common as prevailing, so that @@ -643,7 +646,6 @@ LTO::addRegularLTO(BitcodeModule BM, ArrayRef Syms, CommonRes.Prevailing |= Res.Prevailing; } - // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit. } if (!M.getComdatSymbolTable().empty()) for (GlobalValue &GV : M.global_values()) @@ -698,10 +700,10 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef Syms, assert(ResI != ResE); SymbolResolution Res = *ResI++; - if (Res.Prevailing) { - if (!Sym.getIRName().empty()) { - auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier( - Sym.getIRName(), GlobalValue::ExternalLinkage, "")); + if (!Sym.getIRName().empty()) { + auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier( + Sym.getIRName(), GlobalValue::ExternalLinkage, "")); + if (Res.Prevailing) { ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier(); // For linker redefined symbols (via --wrap or --defsym) we want to @@ -713,6 +715,15 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef Syms, GUID, BM.getModuleIdentifier())) S->setLinkage(GlobalValue::WeakAnyLinkage); } + + // If the linker resolved the symbol to a local definition then mark it + // as local in the summary for the module we are adding. + if (Res.FinalDefinitionInLinkageUnit) { + if (auto S = ThinLTO.CombinedIndex.findSummaryInModule( + GUID, BM.getModuleIdentifier())) { + S->setDSOLocal(true); + } + } } } diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp index fbb61ac..2e6fc4e 100644 --- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -203,6 +203,23 @@ FunctionImportGlobalProcessing::getLinkage(const GlobalValue *SGV, } void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { + + // Check the summaries to see if the symbol gets resolved to a known local + // definition. + if (GV.hasName()) { + ValueInfo VI = ImportIndex.getValueInfo(GV.getGUID()); + if (VI) { + // Need to check all summaries are local in case of hash collisions. + bool IsLocal = VI.getSummaryList().size() && + llvm::all_of(VI.getSummaryList(), + [](const std::unique_ptr &Summary) { + return Summary->isDSOLocal(); + }); + if (IsLocal) + GV.setDSOLocal(true); + } + } + bool DoPromote = false; if (GV.hasLocalLinkage() && ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) { diff --git a/llvm/test/Bitcode/thinlto-summary-local-5.0.ll b/llvm/test/Bitcode/thinlto-summary-local-5.0.ll new file mode 100644 index 0000000..cbc48d2 --- /dev/null +++ b/llvm/test/Bitcode/thinlto-summary-local-5.0.ll @@ -0,0 +1,22 @@ +; Bitcode compatibility test for dso_local flag in thin-lto summaries. +; Checks that older bitcode summaries without the dso_local op are still +; properly parsed and don't set GlobalValues as dso_local. + +; RUN: llvm-dis < %s.bc | FileCheck %s +; RUN: llvm-bcanalyzer -dump %s.bc | FileCheck %s --check-prefix=BCAN + +define void @foo() { +;CHECK-DAG:define void @foo() + ret void +} + +@bar = global i32 0 +;CHECK-DAG: @bar = global i32 0 + +@baz = alias i32, i32* @bar +;CHECK-DAG: @bar = global i32 0 + +;BCAN: +;BCAN-NEXT: +;BCAN-NEXT: diff --git a/llvm/test/Bitcode/thinlto-summary-local-5.0.ll.bc b/llvm/test/Bitcode/thinlto-summary-local-5.0.ll.bc new file mode 100644 index 0000000000000000000000000000000000000000..8dc7ca0a74b760ce63a2967d78da0abefd37c9aa GIT binary patch literal 1028 zcmZ8fZ)h837=N#q=59%LxpqMlc6Z!$Q}V&g<7!#zCD7!w)>$ECei%$K7eh=XO|NN8 z(+nipWxY&;-e!@2{jlg93>7N)L2zK3)EY`0^}k3d3(dMAwFFnu5BuPIai4s6-{*PX zhv)Zu-{((ITG<|Q0B{%p5V7%8VBPXb*!pP*O9Erv(TRgi^S!Wrf73k0VhWEbA#9E$ zf)4gHL;%3q0Qd!3YYzu{pgjUNRycMk&@eq~CWYb4NYVC4FYaPYZ$x=vwMGgo5`PELlrlr;m=Xu4b@jf#b(Em31jEt*vYt-f;z7$x~7D&j2p}R z(+g?m^4iO|HsjP%6>Yv!cx=Bz?B62(S|Rp$G}5S2?JCvm>Mfbt;Hb3%y&;=_)}`Tw zG+gQGFY4Y!$y-9j#ros$(fjAcA5n3+Zu@3ZQ_g59MNP-CY*we%Iub@Q}+AK3p4 z5pA8=8L_umVh?7)y6LS|rWRnn=cug$vvrGVx9HyyCgz;*E=x}?sbVv9{6QjNZ7z#t zD-+`fy4|P%^BS`uQ+GLfdzF3zVT=!RbVoM7a6$Dp>*BKHhXuJ`^R*?h1h4h-*A|$m zHV6NMV>vgLf-M8rVegE&vRYDZSQ9LFMyW`CnA);5yjefLORZNA-whZo+vVv0-U8o| z0GJ^LYx=IL;u0(X>c67;U@`Q%F?TWxfMJiAke(_$8~h)d)Fq_YAO|AFwu60Yf$TeW z{5If((Xj>^jN%i31bGCk9D<=>24djE0f8U~ipUQJRaahuBl!?K^`na)8;*#aicL>P zB literal 0 HcmV?d00001 diff --git a/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll b/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll index f6ee22e..d6022c6 100644 --- a/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll +++ b/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll @@ -17,7 +17,7 @@ ; would clash with the copy from this module. ; RUN: llvm-dis %t3.0.0.preopt.bc -o - | FileCheck %s ; CHECK: define internal void @__cxx_global_var_init() section ".text.startup" { -; CHECK: define available_externally void @testglobfunc() section ".text.startup" { +; CHECK: define available_externally dso_local void @testglobfunc() section ".text.startup" { ; ModuleID = 'comdat-mixed-lto.o' source_filename = "comdat-mixed-lto.cpp" diff --git a/llvm/test/LTO/Resolution/X86/comdat.ll b/llvm/test/LTO/Resolution/X86/comdat.ll index 60d082b..94f2838 100644 --- a/llvm/test/LTO/Resolution/X86/comdat.ll +++ b/llvm/test/LTO/Resolution/X86/comdat.ll @@ -70,14 +70,14 @@ bb11: ; CHECK-DAG: @a23 = alias i32 (i8*), i32 (i8*)* @f1.2{{$}} ; CHECK-DAG: @a24 = alias i16, bitcast (i32 (i8*)* @f1.2 to i16*) -; CHECK: define weak_odr i32 @f1(i8*) comdat($c1) { +; CHECK: define weak_odr dso_local i32 @f1(i8*) comdat($c1) { ; CHECK-NEXT: bb10: ; CHECK-NEXT: br label %bb11{{$}} ; CHECK: bb11: ; CHECK-NEXT: ret i32 42 ; CHECK-NEXT: } -; CHECK: define internal i32 @f1.2(i8* %this) comdat($c2) { +; CHECK: define internal dso_local i32 @f1.2(i8* %this) comdat($c2) { ; CHECK-NEXT: bb20: ; CHECK-NEXT: store i8* %this, i8** null ; CHECK-NEXT: br label %bb21 diff --git a/llvm/test/LTO/Resolution/X86/commons.ll b/llvm/test/LTO/Resolution/X86/commons.ll index 28bf1ad..8adfb87 100644 --- a/llvm/test/LTO/Resolution/X86/commons.ll +++ b/llvm/test/LTO/Resolution/X86/commons.ll @@ -4,7 +4,7 @@ ; RUN: llvm-dis -o - %t.out.0.0.preopt.bc | FileCheck %s ; A strong definition should override the common -; CHECK: @x = global i32 42, align 4 +; CHECK: @x = dso_local global i32 42, align 4 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" diff --git a/llvm/test/ThinLTO/X86/deadstrip.ll b/llvm/test/ThinLTO/X86/deadstrip.ll index c19ccb0..90de3bb 100644 --- a/llvm/test/ThinLTO/X86/deadstrip.ll +++ b/llvm/test/ThinLTO/X86/deadstrip.ll @@ -18,8 +18,8 @@ ; RUN: -r %t2.bc,_boo,pl \ ; 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.0.3.import.bc | FileCheck %s --check-prefix=LTO2 +; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=LTO2-CHECK2 ; RUN: llvm-nm %t.out.1 | FileCheck %s --check-prefix=CHECK2-NM ; RUN: llvm-bcanalyzer -dump %t.out.index.bc | FileCheck %s --check-prefix=COMBINED @@ -27,14 +27,14 @@ ; COMBINED-DAG: