[llvm] r254336 - Start deciding earlier what to link.
Mike Aizatsky via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 30 15:40:15 PST 2015
Rafael,
I believe this brakes sanitizer-windows build:
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/13583
http://lab.llvm.org:8011/builders/sanitizer-windows/builds/13583/steps/run%20tests/logs/stdio
On Mon, Nov 30, 2015 at 2:04 PM Rafael Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: rafael
> Date: Mon Nov 30 16:01:43 2015
> New Revision: 254336
>
> URL: http://llvm.org/viewvc/llvm-project?rev=254336&view=rev
> Log:
> Start deciding earlier what to link.
>
> A traditional linker is roughly split in symbol resolution and "copying
> stuff".
>
> The two tasks are badly mixed in lib/Linker.
>
> This starts splitting them apart.
>
> With this patch there are no direct call to linkGlobalValueBody or
> linkGlobalValueProto. Everything is linked via WapValue.
>
> This also includes a few fixes:
> * A GV goes undefined if the comdat is dropped (comdat11.ll).
> * We error if an internal GV goes undefined (comdat13.ll).
> * We don't link an unused comdat.
>
> The first two match the behavior of an ELF linker. The second one is
> equivalent to running globaldce on the input.
>
> Added:
> llvm/trunk/test/Linker/Inputs/comdat11.ll
> llvm/trunk/test/Linker/Inputs/comdat13.ll
> llvm/trunk/test/Linker/comdat11.ll
> llvm/trunk/test/Linker/comdat12.ll
> llvm/trunk/test/Linker/comdat13.ll
> Modified:
> llvm/trunk/lib/Linker/LinkModules.cpp
> llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> llvm/trunk/test/Linker/comdat9.ll
>
> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=254336&r1=254335&r2=254336&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
> +++ llvm/trunk/lib/Linker/LinkModules.cpp Mon Nov 30 16:01:43 2015
> @@ -436,6 +436,8 @@ class ModuleLinker {
> /// references.
> bool DoneLinkingBodies;
>
> + bool HasError = false;
> +
> public:
> ModuleLinker(Module *dstM, Linker::IdentifiedStructTypeSet &Set, Module
> *srcM,
> DiagnosticHandlerFunction DiagnosticHandler, unsigned
> Flags,
> @@ -483,6 +485,7 @@ private:
> /// Helper method for setting a message and returning an error code.
> bool emitError(const Twine &Message) {
> DiagnosticHandler(LinkDiagnosticInfo(DS_Error, Message));
> + HasError = true;
> return true;
> }
>
> @@ -531,6 +534,7 @@ private:
> void upgradeMismatchedGlobalArray(StringRef Name);
> void upgradeMismatchedGlobals();
>
> + bool linkIfNeeded(GlobalValue &GV);
> bool linkAppendingVarProto(GlobalVariable *DstGV,
> const GlobalVariable *SrcGV);
>
> @@ -904,16 +908,12 @@ Value *ModuleLinker::materializeDeclFor(
> if (doneLinkingBodies())
> return nullptr;
>
> - GlobalValue *DGV = copyGlobalValueProto(TypeMap, SGV);
> -
> - if (Comdat *SC = SGV->getComdat()) {
> - if (auto *DGO = dyn_cast<GlobalObject>(DGV)) {
> - Comdat *DC = DstM->getOrInsertComdat(SC->getName());
> - DGO->setComdat(DC);
> - }
> - }
> -
> - return DGV;
> + linkGlobalValueProto(SGV);
> + if (HasError)
> + return nullptr;
> + Value *Ret = ValueMap[SGV];
> + assert(Ret);
> + return Ret;
> }
>
> void ValueMaterializerTy::materializeInitFor(GlobalValue *New,
> @@ -922,15 +922,31 @@ void ValueMaterializerTy::materializeIni
> }
>
> void ModuleLinker::materializeInitFor(GlobalValue *New, GlobalValue *Old)
> {
> + if (auto *F = dyn_cast<Function>(New)) {
> + if (!F->isDeclaration())
> + return;
> + } else if (auto *V = dyn_cast<GlobalVariable>(New)) {
> + if (V->hasInitializer())
> + return;
> + } else {
> + auto *A = cast<GlobalAlias>(New);
> + if (A->getAliasee())
> + return;
> + }
> +
> + if (Old->isDeclaration())
> + return;
> +
> if (isPerformingImport() && !doImportAsDefinition(Old))
> return;
>
> - // Skip declarations that ValueMaterializer may have created in
> - // case we link in only some of SrcM.
> - if (shouldLinkOnlyNeeded() && Old->isDeclaration())
> + if (DoNotLinkFromSource.count(Old)) {
> + if (!New->hasExternalLinkage() && !New->hasExternalWeakLinkage() &&
> + !New->hasAppendingLinkage())
> + emitError("Declaration points to discarded value");
> return;
> + }
>
> - assert(!Old->isDeclaration() && "users should not pass down decls");
> linkGlobalValueBody(*Old);
> }
>
> @@ -1405,7 +1421,6 @@ bool ModuleLinker::linkGlobalValueProto(
> std::tie(SK, LinkFromSrc) = ComdatsChosen[SC];
> C = DstM->getOrInsertComdat(SC->getName());
> C->setSelectionKind(SK);
> - ComdatMembers[SC].push_back(SGV);
> } else if (DGV) {
> if (shouldLinkFromSource(LinkFromSrc, *DGV, *SGV))
> return true;
> @@ -1425,31 +1440,12 @@ bool ModuleLinker::linkGlobalValueProto(
> if (DGV)
> HasUnnamedAddr = HasUnnamedAddr && DGV->hasUnnamedAddr();
>
> - if (!LinkFromSrc && !DGV)
> - return false;
> -
> GlobalValue *NewGV;
> - if (!LinkFromSrc) {
> + if (!LinkFromSrc && DGV) {
> NewGV = DGV;
> // When linking from source we setVisibility from
> copyGlobalValueProto.
> setVisibility(NewGV, SGV, DGV);
> } else {
> - // If the GV is to be lazily linked, don't create it just yet.
> - // The ValueMaterializerTy will deal with creating it if it's used.
> - if (!DGV && !shouldOverrideFromSrc() && SGV != ImportFunction &&
> - (SGV->hasLocalLinkage() || SGV->hasLinkOnceLinkage() ||
> - SGV->hasAvailableExternallyLinkage())) {
> - DoNotLinkFromSource.insert(SGV);
> - return false;
> - }
> -
> - // When we only want to link in unresolved dependencies, blacklist
> - // the symbol unless unless DestM has a matching declaration (DGV).
> - if (shouldLinkOnlyNeeded() && !(DGV && DGV->isDeclaration())) {
> - DoNotLinkFromSource.insert(SGV);
> - return false;
> - }
> -
> NewGV = copyGlobalValueProto(TypeMap, SGV, DGV);
>
> if (isPerformingImport() && !doImportAsDefinition(SGV))
> @@ -1459,7 +1455,7 @@ bool ModuleLinker::linkGlobalValueProto(
> NewGV->setUnnamedAddr(HasUnnamedAddr);
>
> if (auto *NewGO = dyn_cast<GlobalObject>(NewGV)) {
> - if (C)
> + if (C && LinkFromSrc)
> NewGO->setComdat(C);
>
> if (DGV && DGV->hasCommonLinkage() && SGV->hasCommonLinkage())
> @@ -1842,6 +1838,38 @@ static std::string mergeTriples(const Tr
> return DstTriple.str();
> }
>
> +bool ModuleLinker::linkIfNeeded(GlobalValue &GV) {
> + GlobalValue *DGV = getLinkedToGlobal(&GV);
> +
> + if (shouldLinkOnlyNeeded() && !(DGV && DGV->isDeclaration()))
> + return false;
> +
> + if (DGV && !GV.hasLocalLinkage()) {
> + GlobalValue::VisibilityTypes Visibility =
> + getMinVisibility(DGV->getVisibility(), GV.getVisibility());
> + DGV->setVisibility(Visibility);
> + GV.setVisibility(Visibility);
> + }
> +
> + 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())) {
> + return false;
> + }
> + MapValue(&GV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer);
> + return HasError;
> +}
> +
> bool ModuleLinker::run() {
> assert(DstM && "Null destination module");
> assert(SrcM && "Null source module");
> @@ -1901,24 +1929,30 @@ bool ModuleLinker::run() {
> // Upgrade mismatched global arrays.
> upgradeMismatchedGlobals();
>
> + for (GlobalVariable &GV : SrcM->globals())
> + if (const Comdat *SC = GV.getComdat())
> + ComdatMembers[SC].push_back(&GV);
> +
> + for (Function &SF : *SrcM)
> + if (const Comdat *SC = SF.getComdat())
> + ComdatMembers[SC].push_back(&SF);
> +
> + for (GlobalAlias &GA : SrcM->aliases())
> + if (const Comdat *SC = GA.getComdat())
> + ComdatMembers[SC].push_back(&GA);
> +
> // Insert all of the globals in src into the DstM module... without
> linking
> // initializers (which could refer to functions not yet mapped over).
> for (GlobalVariable &GV : SrcM->globals())
> - if (linkGlobalValueProto(&GV))
> + if (linkIfNeeded(GV))
> return true;
>
> - // Link the functions together between the two modules, without doing
> function
> - // bodies... this just adds external function prototypes to the DstM
> - // function... We do this so that when we begin processing function
> bodies,
> - // all of the global values that may be referenced are available in our
> - // ValueMap.
> - for (Function &F :*SrcM)
> - if (linkGlobalValueProto(&F))
> + for (Function &SF : *SrcM)
> + if (linkIfNeeded(SF))
> return true;
>
> - // If there were any aliases, link them now.
> for (GlobalAlias &GA : SrcM->aliases())
> - if (linkGlobalValueProto(&GA))
> + if (linkIfNeeded(GA))
> return true;
>
> for (AppendingVarInfo &AppendingVar : AppendingVars)
> @@ -1933,37 +1967,6 @@ bool ModuleLinker::run() {
> MapValue(GV, ValueMap, RF_MoveDistinctMDs, &TypeMap,
> &ValMaterializer);
> }
>
> - // Link in the function bodies that are defined in the source module
> into
> - // DstM.
> - for (Function &SF : *SrcM) {
> - // Skip if no body (function is external).
> - if (SF.isDeclaration())
> - continue;
> -
> - // Skip if not linking from source.
> - if (DoNotLinkFromSource.count(&SF))
> - continue;
> -
> - if (linkGlobalValueBody(SF))
> - return true;
> - }
> -
> - // Resolve all uses of aliases with aliasees.
> - for (GlobalAlias &Src : SrcM->aliases()) {
> - if (DoNotLinkFromSource.count(&Src))
> - continue;
> - linkGlobalValueBody(Src);
> - }
> -
> - // Update the initializers in the DstM module now that all globals that
> may
> - // be referenced are in DstM.
> - for (GlobalVariable &Src : SrcM->globals()) {
> - // Only process initialized GV's or ones not already in dest.
> - if (!Src.hasInitializer() || DoNotLinkFromSource.count(&Src))
> - continue;
> - linkGlobalValueBody(Src);
> - }
> -
> // Note that we are done linking global value bodies. This prevents
> // metadata linking from creating new references.
> DoneLinkingBodies = true;
>
> Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=254336&r1=254335&r2=254336&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Mon Nov 30 16:01:43
> 2015
> @@ -41,9 +41,9 @@ Value *llvm::MapValue(const Value *V, Va
> if (Value *NewV =
> Materializer->materializeDeclFor(const_cast<Value *>(V))) {
> VM[V] = NewV;
> - if (auto *GV = dyn_cast<GlobalValue>(V))
> - Materializer->materializeInitFor(cast<GlobalValue>(NewV),
> - const_cast<GlobalValue *>(GV));
> + if (auto *NewGV = dyn_cast<GlobalValue>(NewV))
> + Materializer->materializeInitFor(
> + NewGV, const_cast<GlobalValue *>(cast<GlobalValue>(V)));
> return NewV;
> }
> }
>
> Added: llvm/trunk/test/Linker/Inputs/comdat11.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/comdat11.ll?rev=254336&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/comdat11.ll (added)
> +++ llvm/trunk/test/Linker/Inputs/comdat11.ll Mon Nov 30 16:01:43 2015
> @@ -0,0 +1,9 @@
> +$foo = comdat any
> + at foo = global i8 1, comdat
> +define void @zed() {
> + call void @bar()
> + ret void
> +}
> +define void @bar() comdat($foo) {
> + ret void
> +}
>
> Added: llvm/trunk/test/Linker/Inputs/comdat13.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/comdat13.ll?rev=254336&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/comdat13.ll (added)
> +++ llvm/trunk/test/Linker/Inputs/comdat13.ll Mon Nov 30 16:01:43 2015
> @@ -0,0 +1,9 @@
> +$foo = comdat any
> + at foo = global i8 1, comdat
> +define void @zed() {
> + call void @bar()
> + ret void
> +}
> +define internal void @bar() comdat($foo) {
> + ret void
> +}
>
> Added: llvm/trunk/test/Linker/comdat11.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/comdat11.ll?rev=254336&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/comdat11.ll (added)
> +++ llvm/trunk/test/Linker/comdat11.ll Mon Nov 30 16:01:43 2015
> @@ -0,0 +1,13 @@
> +; RUN: llvm-link -S %s %p/Inputs/comdat11.ll -o - | FileCheck %s
> +
> +$foo = comdat any
> + at foo = global i8 0, comdat
> +
> +; CHECK: @foo = global i8 0, comdat
> +
> +; CHECK: define void @zed() {
> +; CHECK: call void @bar()
> +; CHECK: ret void
> +; CHECK: }
> +
> +; CHECK: declare void @bar()
>
> Added: llvm/trunk/test/Linker/comdat12.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/comdat12.ll?rev=254336&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/comdat12.ll (added)
> +++ llvm/trunk/test/Linker/comdat12.ll Mon Nov 30 16:01:43 2015
> @@ -0,0 +1,8 @@
> +; RUN: llvm-link %s -S -o - | FileCheck %s
> +
> +$foo = comdat largest
> +define internal void @foo() comdat($foo) {
> + ret void
> +}
> +
> +; CHECK-NOT: foo
>
> Added: llvm/trunk/test/Linker/comdat13.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/comdat13.ll?rev=254336&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Linker/comdat13.ll (added)
> +++ llvm/trunk/test/Linker/comdat13.ll Mon Nov 30 16:01:43 2015
> @@ -0,0 +1,13 @@
> +; RUN: not llvm-link -S %s %p/Inputs/comdat13.ll -o %t.ll 2>&1 |
> FileCheck %s
> +
> +; In Inputs/comdat13.ll a function not in the $foo comdat (zed)
> references an
> +; internal function in the comdat $foo.
> +; We might want to have the verifier reject that, but for now we at least
> check
> +; that the linker produces an error.
> +; This is the IR equivalent of the "relocation refers to discarded
> section" in
> +; an ELF linker.
> +
> +; CHECK: Declaration points to discarded value
> +
> +$foo = comdat any
> + at foo = global i8 0, comdat
>
> Modified: llvm/trunk/test/Linker/comdat9.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/comdat9.ll?rev=254336&r1=254335&r2=254336&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Linker/comdat9.ll (original)
> +++ llvm/trunk/test/Linker/comdat9.ll Mon Nov 30 16:01:43 2015
> @@ -14,6 +14,9 @@ $f2 = comdat largest
> define internal void @f2() comdat($f2) {
> ret void
> }
> +define void @f3() comdat($f2) {
> + ret void
> +}
>
> ; CHECK-DAG: $f2 = comdat largest
> ; CHECK-DAG: define internal void @f2() comdat {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151130/6c379443/attachment.html>
More information about the llvm-commits
mailing list