[llvm] r254336 - Start deciding earlier what to link.
Rafael Espindola via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 30 14:01:44 PST 2015
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 {
More information about the llvm-commits
mailing list