From 4744e8bfd8f1eded048dc640661a40cd3e81140b Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Wed, 2 Dec 2015 22:59:04 +0000 Subject: [PATCH] Switch the linker to having a whitelist of GVs. This replaces DoNotLinkFromSource with ValuesToLink. It also moves the computation of ValuesToLink earlier. It is a bit simpler and an important step in slitting the linker into an ir mover and a linker proper. The test change is because we now avoid creating dead declarations. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@254559 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Linker/LinkModules.cpp | 118 +++++++++++++++++++++---------------- test/Linker/funcimport.ll | 8 +-- 2 files changed, 71 insertions(+), 55 deletions(-) diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp index 1a01b78cd0b..21df66863e5 100644 --- a/lib/Linker/LinkModules.cpp +++ b/lib/Linker/LinkModules.cpp @@ -392,8 +392,7 @@ class ModuleLinker { /// but this allows us to reuse the ValueMapper code. ValueToValueMapTy ValueMap; - // Set of items not to link in from source. - SmallPtrSet DoNotLinkFromSource; + SetVector ValuesToLink; DiagnosticHandlerFunction DiagnosticHandler; @@ -862,18 +861,8 @@ Value *ModuleLinker::materializeDeclFor(Value *V) { if (!SGV) return nullptr; - // If we are done linking global value bodies (i.e. we are performing - // metadata linking), don't link in the global value due to this - // reference, simply map it to null. - if (DoneLinkingBodies) - return nullptr; - linkGlobalValueProto(SGV); - if (HasError) - return nullptr; - Value *Ret = ValueMap[SGV]; - assert(Ret); - return Ret; + return ValueMap[SGV]; } void ValueMaterializerTy::materializeInitFor(GlobalValue *New, @@ -881,6 +870,11 @@ void ValueMaterializerTy::materializeInitFor(GlobalValue *New, return ModLinker->materializeInitFor(New, Old); } +static bool shouldLazyLink(const GlobalValue &GV) { + return GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() || + GV.hasAvailableExternallyLinkage(); +} + void ModuleLinker::materializeInitFor(GlobalValue *New, GlobalValue *Old) { if (auto *F = dyn_cast(New)) { if (!F->isDeclaration()) @@ -900,7 +894,7 @@ void ModuleLinker::materializeInitFor(GlobalValue *New, GlobalValue *Old) { if (isPerformingImport() && !doImportAsDefinition(Old)) return; - if (!New->hasLocalLinkage() && DoNotLinkFromSource.count(Old)) + if (!ValuesToLink.count(Old) && !shouldLazyLink(*Old)) return; linkGlobalValueBody(*New, *Old); @@ -1342,7 +1336,8 @@ bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV, [this](Constant *E) { auto *Key = dyn_cast( E->getAggregateElement(2)->stripPointerCasts()); - return DoNotLinkFromSource.count(Key); + return Key && !ValuesToLink.count(Key) && + !shouldLazyLink(*Key); }), SrcElements.end()); uint64_t NewSize = DstElements.size() + SrcElements.size(); @@ -1381,14 +1376,6 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) { // Handle the ultra special appending linkage case first. assert(!DGV || SGV->hasAppendingLinkage() == DGV->hasAppendingLinkage()); - if (SGV->hasAppendingLinkage() && isPerformingImport()) { - // Don't want to append to global_ctors list, for example, when we - // are importing for ThinLTO, otherwise the global ctors and dtors - // get executed multiple times for local variables (the latter causing - // double frees). - DoNotLinkFromSource.insert(SGV); - return false; - } if (SGV->hasAppendingLinkage()) return linkAppendingVarProto(cast_or_null(DGV), cast(SGV)); @@ -1411,15 +1398,9 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) { return true; } - if (!LinkFromSrc) { - // Track the source global so that we don't attempt to copy it over when - // processing global initializers. - DoNotLinkFromSource.insert(SGV); - - if (DGV) - // Make sure to remember this mapping. - ValueMap[SGV] = - ConstantExpr::getBitCast(DGV, TypeMap.get(SGV->getType())); + if (!LinkFromSrc && DGV) { + // Make sure to remember this mapping. + ValueMap[SGV] = ConstantExpr::getBitCast(DGV, TypeMap.get(SGV->getType())); } if (DGV) @@ -1431,6 +1412,12 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) { // When linking from source we setVisibility from copyGlobalValueProto. setVisibility(NewGV, SGV, DGV); } else { + // If we are done linking global value bodies (i.e. we are performing + // metadata linking), don't link in the global value due to this + // reference, simply map it to null. + if (DoneLinkingBodies) + return false; + NewGV = copyGlobalValueProto(TypeMap, SGV, DGV, LinkFromSrc); } @@ -1774,30 +1761,62 @@ bool ModuleLinker::linkIfNeeded(GlobalValue &GV) { if (shouldLinkOnlyNeeded() && !(DGV && DGV->isDeclaration())) return false; - if (DGV && !GV.hasLocalLinkage()) { + if (DGV && !GV.hasLocalLinkage() && !GV.hasAppendingLinkage()) { + auto *DGVar = dyn_cast(DGV); + auto *SGVar = dyn_cast(&GV); + if (DGVar && SGVar) { + if (DGVar->isDeclaration() && SGVar->isDeclaration() && + (!DGVar->isConstant() || !SGVar->isConstant())) { + DGVar->setConstant(false); + SGVar->setConstant(false); + } + if (DGVar->hasCommonLinkage() && SGVar->hasCommonLinkage()) { + unsigned Align = std::max(DGVar->getAlignment(), SGVar->getAlignment()); + SGVar->setAlignment(Align); + DGVar->setAlignment(Align); + } + } + GlobalValue::VisibilityTypes Visibility = getMinVisibility(DGV->getVisibility(), GV.getVisibility()); DGV->setVisibility(Visibility); GV.setVisibility(Visibility); + + bool HasUnnamedAddr = GV.hasUnnamedAddr() && DGV->hasUnnamedAddr(); + DGV->setUnnamedAddr(HasUnnamedAddr); + GV.setUnnamedAddr(HasUnnamedAddr); } + // Don't want to append to global_ctors list, for example, when we + // are importing for ThinLTO, otherwise the global ctors and dtors + // get executed multiple times for local variables (the latter causing + // double frees). + if (GV.hasAppendingLinkage() && isPerformingImport()) + return false; + + if (isPerformingImport() && !doImportAsDefinition(&GV)) + return false; + + if (!DGV && !shouldOverrideFromSrc() && + (GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() || + GV.hasAvailableExternallyLinkage())) + return false; + if (const Comdat *SC = GV.getComdat()) { bool LinkFromSrc; Comdat::SelectionKind SK; std::tie(SK, LinkFromSrc) = ComdatsChosen[SC]; - if (!LinkFromSrc) { - DoNotLinkFromSource.insert(&GV); - return false; - } - } - - if (!DGV && !shouldOverrideFromSrc() && - (GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() || - GV.hasAvailableExternallyLinkage())) { + if (LinkFromSrc) + ValuesToLink.insert(&GV); return false; } - MapValue(&GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); - return HasError; + + bool LinkFromSrc = true; + if (DGV && shouldLinkFromSource(LinkFromSrc, *DGV, GV)) + return true; + if (LinkFromSrc) + ValuesToLink.insert(&GV); + return false; } bool ModuleLinker::run() { @@ -1881,13 +1900,10 @@ bool ModuleLinker::run() { if (linkIfNeeded(GA)) return true; - for (const auto &Entry : DstM.getComdatSymbolTable()) { - const Comdat &C = Entry.getValue(); - if (C.getSelectionKind() == Comdat::Any) - continue; - const GlobalValue *GV = SrcM.getNamedValue(C.getName()); - if (GV) - MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); + for (GlobalValue *GV : ValuesToLink) { + MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer); + if (HasError) + return true; } // Note that we are done linking global value bodies. This prevents diff --git a/test/Linker/funcimport.ll b/test/Linker/funcimport.ll index 1e6c0ec6484..38deafd3e3f 100644 --- a/test/Linker/funcimport.ll +++ b/test/Linker/funcimport.ll @@ -26,20 +26,20 @@ ; lazily linked and never referenced/materialized. ; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=globalfunc1:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB1 ; IMPORTGLOB1-DAG: define available_externally void @globalfunc1 -; IMPORTGLOB1-DAG: declare void @globalfunc2 ; IMPORTGLOB1-DAG: declare void @weakalias ; IMPORTGLOB1-DAG: declare void @analias -; IMPORTGLOB1-DAG: @linkoncealias = external global +; IMPORTGLOB1-NOT: @linkoncealias ; IMPORTGLOB1-NOT: @linkoncefunc +; IMPORTGLOB1-NOT: declare void @globalfunc2 ; Ensure that weak alias to a non-imported function is correctly ; turned into a declaration, but that strong alias to an imported function ; is imported as alias. ; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=globalfunc2:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB2 ; IMPORTGLOB2-DAG: declare void @analias -; IMPORTGLOB2-DAG: declare void @globalfunc1 ; IMPORTGLOB2-DAG: define available_externally void @globalfunc2 ; IMPORTGLOB2-DAG: declare void @weakalias +; IMPORTGLOB2-NOT: declare void @globalfunc1 ; Ensure that strong alias imported in second pass of importing ends up ; as an alias. @@ -98,9 +98,9 @@ ; reference should turned into an external_weak declaration. ; RUN: llvm-link %t2.bc -functionindex=%t3.thinlto.bc -import=callweakfunc:%t.bc -import=weakfunc:%t.bc -S 2>&1 | FileCheck %s --check-prefix=IMPORTWEAKFUNC ; IMPORTWEAKFUNC-DAG: Ignoring import request for weak-any function weakfunc -; IMPORTWEAKFUNC-DAG: @weakvar = extern_weak global i32, align 4 ; IMPORTWEAKFUNC-DAG: declare extern_weak void @weakfunc ; IMPORTWEAKFUNC-DAG: define available_externally void @callweakfunc +; IMPORTWEAKFUNC-NOT: @weakvar = extern_weak global i32, align 4 @globalvar = global i32 1, align 4 @staticvar = internal global i32 1, align 4 -- 2.34.1