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

diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp
index 1a01b78cd0bc5dff98e1b8c7d01fbefd8689a7f3..21df66863e569c7004dce2cf2848b0d5058c1ebb 100644 (file)
--- 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<const GlobalValue *, 16> DoNotLinkFromSource;
+  SetVector<GlobalValue *> 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<Function>(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<GlobalValue>(
                              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<GlobalVariable>(DGV),
                                  cast<GlobalVariable>(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<GlobalVariable>(DGV);
+    auto *SGVar = dyn_cast<GlobalVariable>(&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 1e6c0ec648460eca86d2d349f82ad57597262264..38deafd3e3f16b69adf197e7a9362bf7c2beb7a1 100644 (file)
--- 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