Always pass a diagnostic handler to the linker.
[oota-llvm.git] / lib / Linker / LinkModules.cpp
index c2f472c7e33f5f39834ad5bdca25fd462caa39cc..88b8e443c48930ad7c910ee67ef0a135edfd5a42 100644 (file)
 
 #include "llvm/Linker/Linker.h"
 #include "llvm-c/Linker.h"
-#include "llvm/ADT/Hashing.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/Constants.h"
-#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/TypeFinder.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Debug.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/Cloning.h"
-#include <cctype>
-#include <tuple>
 using namespace llvm;
 
-
 //===----------------------------------------------------------------------===//
 // TypeMap implementation.
 //===----------------------------------------------------------------------===//
@@ -42,22 +32,22 @@ using namespace llvm;
 namespace {
 class TypeMapTy : public ValueMapTypeRemapper {
   /// This is a mapping from a source type to a destination type to use.
-  DenseMap<Type*, Type*> MappedTypes;
+  DenseMap<Type *, Type *> MappedTypes;
 
   /// When checking to see if two subgraphs are isomorphic, we speculatively
   /// add types to MappedTypes, but keep track of them here in case we need to
   /// roll back.
-  SmallVector<Type*, 16> SpeculativeTypes;
+  SmallVector<Type *, 16> SpeculativeTypes;
 
-  SmallVector<StructType*, 16> SpeculativeDstOpaqueTypes;
+  SmallVector<StructType *, 16> SpeculativeDstOpaqueTypes;
 
   /// This is a list of non-opaque structs in the source module that are mapped
   /// to an opaque struct in the destination module.
-  SmallVector<StructType*, 16> SrcDefinitionsToResolve;
+  SmallVector<StructType *, 16> SrcDefinitionsToResolve;
 
   /// This is the set of opaque types in the destination modules who are
   /// getting a body from the source module.
-  SmallPtrSet<StructType*, 16> DstResolvedOpaqueTypes;
+  SmallPtrSet<StructType *, 16> DstResolvedOpaqueTypes;
 
 public:
   TypeMapTy(Linker::IdentifiedStructTypeSet &DstStructTypesSet)
@@ -179,7 +169,7 @@ bool TypeMapTy::areTypesIsomorphic(Type *DstTy, Type *SrcTy) {
 
   // Fail if any of the extra properties (e.g. array size) of the type disagree.
   if (isa<IntegerType>(DstTy))
-    return false;  // bitwidth disagrees.
+    return false; // bitwidth disagrees.
   if (PointerType *PT = dyn_cast<PointerType>(DstTy)) {
     if (PT->getAddressSpace() != cast<PointerType>(SrcTy)->getAddressSpace())
       return false;
@@ -215,7 +205,7 @@ bool TypeMapTy::areTypesIsomorphic(Type *DstTy, Type *SrcTy) {
 }
 
 void TypeMapTy::linkDefinedTypeBodies() {
-  SmallVector<Type*, 16> Elements;
+  SmallVector<Type *, 16> Elements;
   for (StructType *SrcSTy : SrcDefinitionsToResolve) {
     StructType *DstSTy = cast<StructType>(MappedTypes[SrcSTy]);
     assert(DstSTy->isOpaque());
@@ -402,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;
 
@@ -416,18 +405,18 @@ class ModuleLinker {
 
   /// Function to import from source module, all other functions are
   /// imported as declarations instead of definitions.
-  Function *ImportFunction;
+  DenseSet<const GlobalValue *> *ImportFunction;
 
   /// Set to true if the given FunctionInfoIndex contains any functions
   /// from this source module, in which case we must conservatively assume
   /// that any of its functions may be imported into another module
   /// as part of a different backend compilation process.
-  bool HasExportedFunctions;
+  bool HasExportedFunctions = false;
 
   /// Set to true when all global value body linking is complete (including
   /// lazy linking). Used to prevent metadata linking from creating new
   /// references.
-  bool DoneLinkingBodies;
+  bool DoneLinkingBodies = false;
 
   bool HasError = false;
 
@@ -435,11 +424,10 @@ public:
   ModuleLinker(Module &DstM, Linker::IdentifiedStructTypeSet &Set, Module &SrcM,
                DiagnosticHandlerFunction DiagnosticHandler, unsigned Flags,
                const FunctionInfoIndex *Index = nullptr,
-               Function *FuncToImport = nullptr)
+               DenseSet<const GlobalValue *> *FunctionsToImport = nullptr)
       : DstM(DstM), SrcM(SrcM), TypeMap(Set), ValMaterializer(this),
         DiagnosticHandler(DiagnosticHandler), Flags(Flags), ImportIndex(Index),
-        ImportFunction(FuncToImport), HasExportedFunctions(false),
-        DoneLinkingBodies(false) {
+        ImportFunction(FunctionsToImport) {
     assert((ImportIndex || !ImportFunction) &&
            "Expect a FunctionInfoIndex when importing");
     // If we have a FunctionInfoIndex but no function to import,
@@ -447,7 +435,7 @@ public:
     // backend compilation, and we need to see if it has functions that
     // may be exported to another backend compilation.
     if (ImportIndex && !ImportFunction)
-      HasExportedFunctions = ImportIndex->hasExportedFunctions(&SrcM);
+      HasExportedFunctions = ImportIndex->hasExportedFunctions(SrcM);
   }
 
   bool run();
@@ -463,15 +451,12 @@ private:
 
   /// Handles cloning of a global values from the source module into
   /// the destination module, including setting the attributes and visibility.
-  GlobalValue *copyGlobalValueProto(TypeMapTy &TypeMap, const GlobalValue *SGV,
-                                    const GlobalValue *DGV = nullptr);
+  GlobalValue *copyGlobalValueProto(const GlobalValue *SGV,
+                                    const GlobalValue *DGV, bool ForDefinition);
 
   /// Check if we should promote the given local value to global scope.
   bool doPromoteLocalToGlobal(const GlobalValue *SGV);
 
-  /// Check if all global value body linking is complete.
-  bool doneLinkingBodies() { return DoneLinkingBodies; }
-
   bool shouldLinkFromSource(bool &LinkFromSrc, const GlobalValue &Dest,
                             const GlobalValue &Src);
 
@@ -528,23 +513,22 @@ private:
   void upgradeMismatchedGlobals();
 
   bool linkIfNeeded(GlobalValue &GV);
-  bool linkAppendingVarProto(GlobalVariable *DstGV,
-                             const GlobalVariable *SrcGV);
+  Constant *linkAppendingVarProto(GlobalVariable *DstGV,
+                                  const GlobalVariable *SrcGV);
 
-  bool linkGlobalValueProto(GlobalValue *GV);
+  Constant *linkGlobalValueProto(GlobalValue *GV);
   bool linkModuleFlagsMetadata();
 
   void linkGlobalInit(GlobalVariable &Dst, GlobalVariable &Src);
   bool linkFunctionBody(Function &Dst, Function &Src);
   void linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src);
-  bool linkGlobalValueBody(GlobalValue &Src);
+  bool linkGlobalValueBody(GlobalValue &Dst, GlobalValue &Src);
 
   /// Functions that take care of cloning a specific global value type
   /// into the destination module.
-  GlobalVariable *copyGlobalVariableProto(TypeMapTy &TypeMap,
-                                          const GlobalVariable *SGVar);
-  Function *copyFunctionProto(TypeMapTy &TypeMap, const Function *SF);
-  GlobalValue *copyGlobalAliasProto(TypeMapTy &TypeMap, const GlobalAlias *SGA);
+  GlobalVariable *copyGlobalVariableProto(const GlobalVariable *SGVar);
+  Function *copyFunctionProto(const Function *SF);
+  GlobalValue *copyGlobalAliasProto(const GlobalAlias *SGA);
 
   /// Helper methods to check if we are importing from or potentially
   /// exporting from the current source module.
@@ -596,10 +580,10 @@ static void forceRenaming(GlobalValue *GV, StringRef Name) {
   // If there is a conflict, rename the conflict.
   if (GlobalValue *ConflictGV = M->getNamedValue(Name)) {
     GV->takeName(ConflictGV);
-    ConflictGV->setName(Name);    // This will cause ConflictGV to get renamed
+    ConflictGV->setName(Name); // This will cause ConflictGV to get renamed
     assert(ConflictGV->getName() != Name && "forceRenaming didn't work");
   } else {
-    GV->setName(Name);              // Force the name back
+    GV->setName(Name); // Force the name back
   }
 }
 
@@ -607,18 +591,7 @@ static void forceRenaming(GlobalValue *GV, StringRef Name) {
 /// from the SrcGV to the DestGV.
 void ModuleLinker::copyGVAttributes(GlobalValue *NewGV,
                                     const GlobalValue *SrcGV) {
-  auto *GA = dyn_cast<GlobalAlias>(SrcGV);
-  // Check for the special case of converting an alias (definition) to a
-  // non-alias (declaration). This can happen when we are importing and
-  // encounter a weak_any alias (weak_any defs may not be imported, see
-  // comments in ModuleLinker::getLinkage) or an alias whose base object is
-  // being imported as a declaration. In that case copy the attributes from the
-  // base object.
-  if (GA && !dyn_cast<GlobalAlias>(NewGV)) {
-    assert(isPerformingImport() && !doImportAsDefinition(GA));
-    NewGV->copyAttributesFrom(GA->getBaseObject());
-  } else
-    NewGV->copyAttributesFrom(SrcGV);
+  NewGV->copyAttributesFrom(SrcGV);
   forceRenaming(NewGV, getName(SrcGV));
 }
 
@@ -646,7 +619,7 @@ bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
     return true;
   // Only import the function requested for importing.
   auto *SF = dyn_cast<Function>(SGV);
-  if (SF && SF == ImportFunction)
+  if (SF && ImportFunction->count(SF))
     return true;
   // Otherwise no.
   return false;
@@ -753,8 +726,10 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
     // It would be incorrect to import an appending linkage variable,
     // since it would cause global constructors/destructors to be
     // executed multiple times. This should have already been handled
-    // by linkGlobalValueProto.
-    llvm_unreachable("Cannot import appending linkage variable");
+    // by linkIfNeeded, and we will assert in shouldLinkFromSource
+    // if we try to import, so we simply return AppendingLinkage here
+    // as this helper is called more widely in getLinkedToGlobal.
+    return GlobalValue::AppendingLinkage;
 
   case GlobalValue::InternalLinkage:
   case GlobalValue::PrivateLinkage:
@@ -788,62 +763,36 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
 /// Loop through the global variables in the src module and merge them into the
 /// dest module.
 GlobalVariable *
-ModuleLinker::copyGlobalVariableProto(TypeMapTy &TypeMap,
-                                      const GlobalVariable *SGVar) {
+ModuleLinker::copyGlobalVariableProto(const GlobalVariable *SGVar) {
   // No linking to be performed or linking from the source: simply create an
   // identical version of the symbol over in the dest module... the
   // initializer will be filled in later by LinkGlobalInits.
-  GlobalVariable *NewDGV = new GlobalVariable(
-      DstM, TypeMap.get(SGVar->getType()->getElementType()),
-      SGVar->isConstant(), getLinkage(SGVar), /*init*/ nullptr, getName(SGVar),
-      /*insertbefore*/ nullptr, SGVar->getThreadLocalMode(),
-      SGVar->getType()->getAddressSpace());
+  GlobalVariable *NewDGV =
+      new GlobalVariable(DstM, TypeMap.get(SGVar->getType()->getElementType()),
+                         SGVar->isConstant(), GlobalValue::ExternalLinkage,
+                         /*init*/ nullptr, getName(SGVar),
+                         /*insertbefore*/ nullptr, SGVar->getThreadLocalMode(),
+                         SGVar->getType()->getAddressSpace());
 
   return NewDGV;
 }
 
 /// Link the function in the source module into the destination module if
 /// needed, setting up mapping information.
-Function *ModuleLinker::copyFunctionProto(TypeMapTy &TypeMap,
-                                          const Function *SF) {
+Function *ModuleLinker::copyFunctionProto(const Function *SF) {
   // If there is no linkage to be performed or we are linking from the source,
   // bring SF over.
-  return Function::Create(TypeMap.get(SF->getFunctionType()), getLinkage(SF),
-                          getName(SF), &DstM);
+  return Function::Create(TypeMap.get(SF->getFunctionType()),
+                          GlobalValue::ExternalLinkage, getName(SF), &DstM);
 }
 
 /// Set up prototypes for any aliases that come over from the source module.
-GlobalValue *ModuleLinker::copyGlobalAliasProto(TypeMapTy &TypeMap,
-                                                const GlobalAlias *SGA) {
-  // If we are importing and encounter a weak_any alias, or an alias to
-  // an object being imported as a declaration, we must import the alias
-  // as a declaration as well, which involves converting it to a non-alias.
-  // See comments in ModuleLinker::getLinkage for why we cannot import
-  // weak_any defintions.
-  if (isPerformingImport() && !doImportAsDefinition(SGA)) {
-    // Need to convert to declaration. All aliases must be definitions.
-    const GlobalValue *GVal = SGA->getBaseObject();
-    GlobalValue *NewGV;
-    if (auto *GVar = dyn_cast<GlobalVariable>(GVal))
-      NewGV = copyGlobalVariableProto(TypeMap, GVar);
-    else {
-      auto *F = dyn_cast<Function>(GVal);
-      assert(F);
-      NewGV = copyFunctionProto(TypeMap, F);
-    }
-    // Set the linkage to External or ExternalWeak (see comments in
-    // ModuleLinker::getLinkage for why WeakAny is converted to ExternalWeak).
-    if (SGA->hasWeakAnyLinkage())
-      NewGV->setLinkage(GlobalValue::ExternalWeakLinkage);
-    else
-      NewGV->setLinkage(GlobalValue::ExternalLinkage);
-    return NewGV;
-  }
+GlobalValue *ModuleLinker::copyGlobalAliasProto(const GlobalAlias *SGA) {
   // If there is no linkage to be performed or we're linking from the source,
   // bring over SGA.
   auto *Ty = TypeMap.get(SGA->getValueType());
   return GlobalAlias::create(Ty, SGA->getType()->getPointerAddressSpace(),
-                             getLinkage(SGA), getName(SGA), &DstM);
+                             GlobalValue::ExternalLinkage, getName(SGA), &DstM);
 }
 
 static GlobalValue::VisibilityTypes
@@ -869,16 +818,32 @@ void ModuleLinker::setVisibility(GlobalValue *NewGV, const GlobalValue *SGV,
   NewGV->setVisibility(Visibility);
 }
 
-GlobalValue *ModuleLinker::copyGlobalValueProto(TypeMapTy &TypeMap,
-                                                const GlobalValue *SGV,
-                                                const GlobalValue *DGV) {
+GlobalValue *ModuleLinker::copyGlobalValueProto(const GlobalValue *SGV,
+                                                const GlobalValue *DGV,
+                                                bool ForDefinition) {
   GlobalValue *NewGV;
-  if (auto *SGVar = dyn_cast<GlobalVariable>(SGV))
-    NewGV = copyGlobalVariableProto(TypeMap, SGVar);
-  else if (auto *SF = dyn_cast<Function>(SGV))
-    NewGV = copyFunctionProto(TypeMap, SF);
-  else
-    NewGV = copyGlobalAliasProto(TypeMap, cast<GlobalAlias>(SGV));
+  if (auto *SGVar = dyn_cast<GlobalVariable>(SGV)) {
+    NewGV = copyGlobalVariableProto(SGVar);
+  } else if (auto *SF = dyn_cast<Function>(SGV)) {
+    NewGV = copyFunctionProto(SF);
+  } else {
+    if (ForDefinition)
+      NewGV = copyGlobalAliasProto(cast<GlobalAlias>(SGV));
+    else
+      NewGV = new GlobalVariable(
+          DstM, TypeMap.get(SGV->getType()->getElementType()),
+          /*isConstant*/ false, GlobalValue::ExternalLinkage,
+          /*init*/ nullptr, getName(SGV),
+          /*insertbefore*/ nullptr, SGV->getThreadLocalMode(),
+          SGV->getType()->getAddressSpace());
+  }
+
+  if (ForDefinition)
+    NewGV->setLinkage(getLinkage(SGV));
+  else if (SGV->hasAvailableExternallyLinkage() || SGV->hasWeakLinkage() ||
+           SGV->hasLinkOnceLinkage())
+    NewGV->setLinkage(GlobalValue::ExternalWeakLinkage);
+
   copyGVAttributes(NewGV, SGV);
   setVisibility(NewGV, SGV, DGV);
   return NewGV;
@@ -893,18 +858,7 @@ 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 linkGlobalValueProto(SGV);
 }
 
 void ValueMaterializerTy::materializeInitFor(GlobalValue *New,
@@ -912,6 +866,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())
@@ -931,10 +890,10 @@ 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(*Old);
+  linkGlobalValueBody(*New, *Old);
 }
 
 bool ModuleLinker::getComdatLeader(Module &M, StringRef ComdatName,
@@ -1058,8 +1017,7 @@ bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc,
 
   // We always have to add Src if it has appending linkage.
   if (Src.hasAppendingLinkage()) {
-    // Caller should have already determined that we can't link from source
-    // when importing (see comments in linkGlobalValueProto).
+    // Should have prevented importing for appending linkage in linkIfNeeded.
     assert(!isPerformingImport());
     LinkFromSrc = true;
     return false;
@@ -1072,7 +1030,7 @@ bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc,
     if (isa<Function>(&Src)) {
       // For functions, LinkFromSrc iff this is the function requested
       // for importing. For variables, decide below normally.
-      LinkFromSrc = (&Src == ImportFunction);
+      LinkFromSrc = ImportFunction->count(&Src);
       return false;
     }
 
@@ -1319,8 +1277,8 @@ static void getArrayElements(const Constant *C,
 
 /// If there were any appending global variables, link them together now.
 /// Return true on error.
-bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV,
-                                         const GlobalVariable *SrcGV) {
+Constant *ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV,
+                                              const GlobalVariable *SrcGV) {
   ArrayType *SrcTy =
       cast<ArrayType>(TypeMap.get(SrcGV->getType()->getElementType()));
   Type *EltTy = SrcTy->getElementType();
@@ -1328,32 +1286,46 @@ bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV,
   if (DstGV) {
     ArrayType *DstTy = cast<ArrayType>(DstGV->getType()->getElementType());
 
-    if (!SrcGV->hasAppendingLinkage() || !DstGV->hasAppendingLinkage())
-      return emitError(
+    if (!SrcGV->hasAppendingLinkage() || !DstGV->hasAppendingLinkage()) {
+      emitError(
           "Linking globals named '" + SrcGV->getName() +
           "': can only link appending global with another appending global!");
+      return nullptr;
+    }
 
     // Check to see that they two arrays agree on type.
-    if (EltTy != DstTy->getElementType())
-      return emitError("Appending variables with different element types!");
-    if (DstGV->isConstant() != SrcGV->isConstant())
-      return emitError("Appending variables linked with different const'ness!");
+    if (EltTy != DstTy->getElementType()) {
+      emitError("Appending variables with different element types!");
+      return nullptr;
+    }
+    if (DstGV->isConstant() != SrcGV->isConstant()) {
+      emitError("Appending variables linked with different const'ness!");
+      return nullptr;
+    }
 
-    if (DstGV->getAlignment() != SrcGV->getAlignment())
-      return emitError(
+    if (DstGV->getAlignment() != SrcGV->getAlignment()) {
+      emitError(
           "Appending variables with different alignment need to be linked!");
+      return nullptr;
+    }
 
-    if (DstGV->getVisibility() != SrcGV->getVisibility())
-      return emitError(
+    if (DstGV->getVisibility() != SrcGV->getVisibility()) {
+      emitError(
           "Appending variables with different visibility need to be linked!");
+      return nullptr;
+    }
 
-    if (DstGV->hasUnnamedAddr() != SrcGV->hasUnnamedAddr())
-      return emitError(
+    if (DstGV->hasUnnamedAddr() != SrcGV->hasUnnamedAddr()) {
+      emitError(
           "Appending variables with different unnamed_addr need to be linked!");
+      return nullptr;
+    }
 
-    if (StringRef(DstGV->getSection()) != SrcGV->getSection())
-      return emitError(
+    if (StringRef(DstGV->getSection()) != SrcGV->getSection()) {
+      emitError(
           "Appending variables with different section name need to be linked!");
+      return nullptr;
+    }
   }
 
   SmallVector<Constant *, 16> DstElements;
@@ -1373,7 +1345,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();
@@ -1388,9 +1361,10 @@ bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV,
   // Propagate alignment, visibility and section info.
   copyGVAttributes(NG, SrcGV);
 
-  // Replace any uses of the two global variables with uses of the new
-  // global.
-  ValueMap[SrcGV] = ConstantExpr::getBitCast(NG, TypeMap.get(SrcGV->getType()));
+  Constant *Ret = ConstantExpr::getBitCast(NG, TypeMap.get(SrcGV->getType()));
+
+  // Stop recursion.
+  ValueMap[SrcGV] = Ret;
 
   for (auto *V : SrcElements) {
     DstElements.push_back(
@@ -1399,56 +1373,44 @@ bool ModuleLinker::linkAppendingVarProto(GlobalVariable *DstGV,
 
   NG->setInitializer(ConstantArray::get(NewType, DstElements));
 
+  // Replace any uses of the two global variables with uses of the new
+  // global.
   if (DstGV) {
     DstGV->replaceAllUsesWith(ConstantExpr::getBitCast(NG, DstGV->getType()));
     DstGV->eraseFromParent();
   }
 
-  return false;
+  return Ret;
 }
 
-bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
+Constant *ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
   GlobalValue *DGV = getLinkedToGlobal(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())
+  if (SGV->hasAppendingLinkage()) {
+    // Should have prevented importing for appending linkage in linkIfNeeded.
+    assert(!isPerformingImport());
     return linkAppendingVarProto(cast_or_null<GlobalVariable>(DGV),
                                  cast<GlobalVariable>(SGV));
+  }
 
   bool LinkFromSrc = true;
   Comdat *C = nullptr;
   bool HasUnnamedAddr = SGV->hasUnnamedAddr();
 
-  if (const Comdat *SC = SGV->getComdat()) {
+  if (isPerformingImport() && !doImportAsDefinition(SGV)) {
+    LinkFromSrc = false;
+  } else if (const Comdat *SC = SGV->getComdat()) {
     Comdat::SelectionKind SK;
     std::tie(SK, LinkFromSrc) = ComdatsChosen[SC];
     C = DstM.getOrInsertComdat(SC->getName());
     C->setSelectionKind(SK);
-    if (SGV->hasInternalLinkage())
+    if (SGV->hasLocalLinkage())
       LinkFromSrc = true;
   } else if (DGV) {
     if (shouldLinkFromSource(LinkFromSrc, *DGV, *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()));
+      return nullptr;
   }
 
   if (DGV)
@@ -1460,10 +1422,13 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
     // When linking from source we setVisibility from copyGlobalValueProto.
     setVisibility(NewGV, SGV, DGV);
   } else {
-    NewGV = copyGlobalValueProto(TypeMap, SGV, DGV);
+    // 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;
 
-    if (isPerformingImport() && !doImportAsDefinition(SGV))
-      DoNotLinkFromSource.insert(SGV);
+    NewGV = copyGlobalValueProto(SGV, DGV, LinkFromSrc);
   }
 
   NewGV->setUnnamedAddr(HasUnnamedAddr);
@@ -1484,16 +1449,12 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
       NewGVar->setConstant(false);
   }
 
-  // Make sure to remember this mapping.
-  if (NewGV != DGV) {
-    if (DGV) {
-      DGV->replaceAllUsesWith(ConstantExpr::getBitCast(NewGV, DGV->getType()));
-      DGV->eraseFromParent();
-    }
-    ValueMap[SGV] = NewGV;
+  if (NewGV != DGV && DGV) {
+    DGV->replaceAllUsesWith(ConstantExpr::getBitCast(NewGV, DGV->getType()));
+    DGV->eraseFromParent();
   }
 
-  return false;
+  return ConstantExpr::getBitCast(NewGV, TypeMap.get(SGV->getType()));
 }
 
 /// Update the initializers in the Dest module now that all globals that may be
@@ -1534,7 +1495,7 @@ bool ModuleLinker::linkFunctionBody(Function &Dst, Function &Src) {
   // Go through and convert function arguments over, remembering the mapping.
   Function::arg_iterator DI = Dst.arg_begin();
   for (Argument &Arg : Src.args()) {
-    DI->setName(Arg.getName());  // Copy the name over.
+    DI->setName(Arg.getName()); // Copy the name over.
 
     // Add a mapping to our mapping.
     ValueMap[&Arg] = &*DI;
@@ -1576,9 +1537,7 @@ void ModuleLinker::linkAliasBody(GlobalAlias &Dst, GlobalAlias &Src) {
   Dst.setAliasee(Val);
 }
 
-bool ModuleLinker::linkGlobalValueBody(GlobalValue &Src) {
-  Value *Dst = ValueMap[&Src];
-  assert(Dst);
+bool ModuleLinker::linkGlobalValueBody(GlobalValue &Dst, GlobalValue &Src) {
   if (const Comdat *SC = Src.getComdat()) {
     // To ensure that we don't generate an incomplete comdat group,
     // we must materialize and map in any other members that are not
@@ -1586,22 +1545,22 @@ bool ModuleLinker::linkGlobalValueBody(GlobalValue &Src) {
     // are linked in. Otherwise, linkonce and other lazy linked GVs will
     // not be materialized if they aren't referenced.
     for (auto *SGV : ComdatMembers[SC]) {
-      auto *DGV = cast_or_null<GlobalValue>(ValueMap[SGV]);
+      auto *DGV = cast_or_null<GlobalValue>(ValueMap.lookup(SGV));
       if (DGV && !DGV->isDeclaration())
         continue;
       MapValue(SGV, ValueMap, RF_MoveDistinctMDs, &TypeMap, &ValMaterializer);
     }
   }
   if (shouldInternalizeLinkedSymbols())
-    if (auto *DGV = dyn_cast<GlobalValue>(Dst))
+    if (auto *DGV = dyn_cast<GlobalValue>(&Dst))
       DGV->setLinkage(GlobalValue::InternalLinkage);
   if (auto *F = dyn_cast<Function>(&Src))
-    return linkFunctionBody(cast<Function>(*Dst), *F);
+    return linkFunctionBody(cast<Function>(Dst), *F);
   if (auto *GVar = dyn_cast<GlobalVariable>(&Src)) {
-    linkGlobalInit(cast<GlobalVariable>(*Dst), *GVar);
+    linkGlobalInit(cast<GlobalVariable>(Dst), *GVar);
     return false;
   }
-  linkAliasBody(cast<GlobalAlias>(*Dst), cast<GlobalAlias>(Src));
+  linkAliasBody(cast<GlobalAlias>(Dst), cast<GlobalAlias>(Src));
   return false;
 }
 
@@ -1625,7 +1584,8 @@ void ModuleLinker::linkNamedMDNodes() {
 bool ModuleLinker::linkModuleFlagsMetadata() {
   // If the source module has no module flags, we are done.
   const NamedMDNode *SrcModFlags = SrcM.getModuleFlagsMetadata();
-  if (!SrcModFlags) return false;
+  if (!SrcModFlags)
+    return false;
 
   // If the destination module doesn't have module flags yet, then just copy
   // over the source module's flags.
@@ -1639,7 +1599,7 @@ bool ModuleLinker::linkModuleFlagsMetadata() {
 
   // First build a map of the existing module flags and requirements.
   DenseMap<MDString *, std::pair<MDNode *, unsigned>> Flags;
-  SmallSetVector<MDNode*, 16> Requirements;
+  SmallSetVector<MDNode *, 16> Requirements;
   for (unsigned I = 0, E = DstModFlags->getNumOperands(); I != E; ++I) {
     MDNode *Op = DstModFlags->getOperand(I);
     ConstantInt *Behavior = mdconst::extract<ConstantInt>(Op->getOperand(0));
@@ -1720,7 +1680,8 @@ bool ModuleLinker::linkModuleFlagsMetadata() {
     // Perform the merge for standard behavior types.
     switch (SrcBehaviorValue) {
     case Module::Require:
-    case Module::Override: llvm_unreachable("not possible");
+    case Module::Override:
+      llvm_unreachable("not possible");
     case Module::Error: {
       // Emit an error if the values differ.
       if (SrcOp->getOperand(2) != DstOp->getOperand(2)) {
@@ -1783,16 +1744,15 @@ bool ModuleLinker::linkModuleFlagsMetadata() {
 static bool triplesMatch(const Triple &T0, const Triple &T1) {
   // If vendor is apple, ignore the version number.
   if (T0.getVendor() == Triple::Apple)
-    return T0.getArch() == T1.getArch() &&
-           T0.getSubArch() == T1.getSubArch() &&
-           T0.getVendor() == T1.getVendor() &&
-           T0.getOS() == T1.getOS();
+    return T0.getArch() == T1.getArch() && T0.getSubArch() == T1.getSubArch() &&
+           T0.getVendor() == T1.getVendor() && T0.getOS() == T1.getOS();
 
   return T0 == T1;
 }
 
 // This function returns the merged triple.
-static std::string mergeTriples(const Triple &SrcTriple, const Triple &DstTriple) {
+static std::string mergeTriples(const Triple &SrcTriple,
+                                const Triple &DstTriple) {
   // If vendor is apple, pick the triple with the larger version number.
   if (SrcTriple.getVendor() == Triple::Apple)
     if (DstTriple.isOSVersionLT(SrcTriple))
@@ -1807,30 +1767,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() {
@@ -1914,13 +1906,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
@@ -2041,16 +2030,11 @@ Linker::Linker(Module &M, DiagnosticHandlerFunction DiagnosticHandler)
   }
 }
 
-Linker::Linker(Module &M)
-    : Linker(M, [this](const DiagnosticInfo &DI) {
-        Composite.getContext().diagnose(DI);
-      }) {}
-
 bool Linker::linkInModule(Module &Src, unsigned Flags,
                           const FunctionInfoIndex *Index,
-                          Function *FuncToImport) {
+                          DenseSet<const GlobalValue *> *FunctionsToImport) {
   ModuleLinker TheLinker(Composite, IdentifiedStructTypes, Src,
-                         DiagnosticHandler, Flags, Index, FuncToImport);
+                         DiagnosticHandler, Flags, Index, FunctionsToImport);
   bool RetCode = TheLinker.run();
   Composite.dropTriviallyDeadConstantArrays();
   return RetCode;
@@ -2072,11 +2056,6 @@ bool Linker::linkModules(Module &Dest, Module &Src,
   return L.linkInModule(Src, Flags);
 }
 
-bool Linker::linkModules(Module &Dest, Module &Src, unsigned Flags) {
-  Linker L(Dest);
-  return L.linkInModule(Src, Flags);
-}
-
 //===----------------------------------------------------------------------===//
 // C API.
 //===----------------------------------------------------------------------===//