From 39770ca0a11c4cd19c38f51a1ffda6b07ab8b90d Mon Sep 17 00:00:00 2001 From: Sean Fertile Date: Sat, 4 Nov 2017 01:54:20 +0000 Subject: [PATCH] Revert "[LTO][ThinLTO] Use the linker resolutions to mark global values ..." Changes more tests then expected on one of the build bots. reverting to investigate. This reverts https://llvm.org/svn/llvm-project/llvm/trunk@317374 llvm-svn: 317395 --- 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 1028 -> 0 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 - 19 files changed, 37 insertions(+), 115 deletions(-) delete mode 100644 llvm/test/Bitcode/thinlto-summary-local-5.0.ll delete 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 b1e58a2..2d664f4 100644 --- a/llvm/include/llvm/IR/ModuleSummaryIndex.h +++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h @@ -148,15 +148,11 @@ 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 IsLocal) + bool NotEligibleToImport, bool Live) : Linkage(Linkage), NotEligibleToImport(NotEligibleToImport), - Live(Live), DSOLocal(IsLocal) {} + Live(Live) {} }; private: @@ -233,10 +229,6 @@ 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 4687f2d..2f9990c 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, IsLocal; + bool NotEligibleToImport, Live; std::vector TypeTests; std::vector TypeTestAssumeVCalls, TypeCheckedLoadVCalls; @@ -177,7 +177,6 @@ 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); @@ -212,7 +211,7 @@ template <> struct CustomMappingTraits { Elem.SummaryList.push_back(llvm::make_unique( GlobalValueSummary::GVFlags( static_cast(FSum.Linkage), - FSum.NotEligibleToImport, FSum.Live, FSum.IsLocal), + FSum.NotEligibleToImport, FSum.Live), 0, FunctionSummary::FFlags{}, ArrayRef{}, ArrayRef{}, std::move(FSum.TypeTests), std::move(FSum.TypeTestAssumeVCalls), @@ -229,8 +228,7 @@ template <> struct CustomMappingTraits { FSums.push_back(FunctionSummaryYaml{ FSum->flags().Linkage, static_cast(FSum->flags().NotEligibleToImport), - static_cast(FSum->flags().Live), - static_cast(FSum->flags().DSOLocal), FSum->type_tests(), + static_cast(FSum->flags().Live), 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 82db09c..afd575e 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, F.isDSOLocal()); + /* Live = */ false); 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, V.isDSOLocal()); + /* Live = */ false); 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, A.isDSOLocal()); + /* Live = */ false); auto AS = llvm::make_unique(Flags); auto *Aliasee = A.getBaseObject(); auto *AliaseeSummary = Index.getGlobalValueSummary(*Aliasee); @@ -410,8 +410,7 @@ ModuleSummaryIndex llvm::buildModuleSummaryIndex( assert(GV->isDeclaration() && "Def in module asm already has definition"); GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage, /* NotEligibleToImport = */ true, - /* Live = */ true, - /* Local */ GV->isDSOLocal()); + /* Live = */ true); 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 d0f11db..c227226 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -889,9 +889,7 @@ 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; - bool Local = (RawFlags & 0x4); - - return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live, Local); + return GlobalValueSummary::GVFlags(Linkage, NotEligibleToImport, Live); } static GlobalValue::VisibilityTypes getDecodedVisibility(unsigned Val) { diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index c5d376c..1e491aa 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -955,8 +955,6 @@ 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 9c73779..017dd20 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -630,9 +630,6 @@ 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 @@ -646,6 +643,7 @@ 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()) @@ -700,10 +698,10 @@ Error LTO::addThinLTO(BitcodeModule BM, ArrayRef Syms, assert(ResI != ResE); SymbolResolution Res = *ResI++; - if (!Sym.getIRName().empty()) { - auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier( - Sym.getIRName(), GlobalValue::ExternalLinkage, "")); - if (Res.Prevailing) { + if (Res.Prevailing) { + if (!Sym.getIRName().empty()) { + auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier( + Sym.getIRName(), GlobalValue::ExternalLinkage, "")); ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier(); // For linker redefined symbols (via --wrap or --defsym) we want to @@ -715,15 +713,6 @@ 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 2e6fc4e..fbb61ac 100644 --- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -203,23 +203,6 @@ 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 deleted file mode 100644 index cbc48d2..0000000 --- a/llvm/test/Bitcode/thinlto-summary-local-5.0.ll +++ /dev/null @@ -1,22 +0,0 @@ -; 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 deleted file mode 100644 index 8dc7ca0a74b760ce63a2967d78da0abefd37c9aa..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 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 diff --git a/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll b/llvm/test/LTO/Resolution/X86/comdat-mixed-lto.ll index d6022c6..f6ee22e 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 dso_local void @testglobfunc() section ".text.startup" { +; CHECK: define available_externally 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 94f2838..60d082b 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 dso_local i32 @f1(i8*) comdat($c1) { +; CHECK: define weak_odr 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 dso_local i32 @f1.2(i8* %this) comdat($c2) { +; CHECK: define internal 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 8adfb87..28bf1ad 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 = dso_local global i32 42, align 4 +; CHECK: @x = 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 90de3bb..c19ccb0 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 --check-prefix=LTO2 -; RUN: llvm-dis < %t.out.1.3.import.bc | FileCheck %s --check-prefix=LTO2-CHECK2 +; 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-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: