Rework linkInModule(), making it oblivious to ThinLTO

[mehdi_amini]

Summary

ThinLTO is relying on linkInModule to import selected function.
However a lot of "magic" was hidden in linkInModule and the IRMover,
who would rename and promote global variables on the fly.

This is moving to an approach where the steps are decoupled and the
client is reponsible to specify the list of globals to import.
As a consequence some test are changed because they were relying on
the previous behavior which was importing the definition of *every*
single global without control on the client side.
Now the burden is on the client to decide if a global has to be imported
or not.

[tejohnson]

This is a nice cleanup and less invasive than I feared. I am concerned as noted below about losing all variable definition importing. Will try to apply this and do a performance comparison with spec to see if there are currently any effects.

lib/Linker/LinkModules.cpp

404–405

This seems like it should be redundant/combined with the condition added to the below if statement. Can't the below if statement just be "if (!isPerformingImport() && !DGV && ...)"? I.e. not need to check GlobalsToImport again since it was already checked in doImportAsDefinition, and if we reach the below if statement when isPerformingImport() then we do want this symbol?

Alternatively, you could combine this and the below if into one block like:

if (isPerformingImport()) {

if (!doImportAsDefinition(&GV))
    return false;

} else if (!DGV && ...)

return false;

lib/Transforms/IPO/FunctionImport.cpp

287

Moving this is unrelated to the rest of the patch?

355

Rename FunctionsToImport to GlobalsToImport here as well?

Although currently with this change it looks like we will never import a global variable definition since the importer is only adding functions to this list, right? I wonder if there is a performance loss. Will apply this and try with spec.

test/Transforms/FunctionImport/Inputs/funcimport.ll

23 ↗

(On Diff #50528)

Why was globalfunc3 and staticvar2 added? Don't see anything checking them in the test.

tools/llvm-link/llvm-link.cpp

212

Extend and invoke llvm::renameModuleForThinLTO instead. Ditto for similar block added to FunctionImport.cpp.

282

Use llvm::renameModuleForThinLTO instead

[mehdi_amini]

Update to take Teresa's comments into account.

[mehdi_amini]

Update tests as well

lib/Transforms/IPO/FunctionImport.cpp

355

Yes no import of globals yet, we operate only on functions. It makes sense only for constant, right? I don't know if we have this information in the summary?

test/Transforms/FunctionImport/Inputs/funcimport.ll

23 ↗

(On Diff #50528)

This is a partial leftover from further changes that haven't been submitted yet (testing that we promote only what is imported elsewhere). Will remove from this patch.

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

355

Ran spec with this patch applied, no significant performance delta, so this is not currently a big issue.

Might have to augment the summary again to note if they are constant.

442

Use a default on the new parameter of nullptr instead of passing it explicitly?

lib/Transforms/Utils/FunctionImportUtils.cpp

227

Shouldn't the getNewExportedValues be added to GlobalsToImport if non-null?

tools/llvm-link/llvm-link.cpp

292–293

I thought I removed the PreserveModules code from HEAD awhile back? How did it get merged back?

[• rafael]

I really like this change, but Teresa should be the one to give the final LGTM.

include/llvm/Linker/Linker.h

49

Can you remove the llvm/IR/ModuleSummaryIndex.h include?

[mehdi_amini]

Rebase on top of trunk. And remove the include pointed by Rafael.

[mehdi_amini]

Add a default value for renameModuleForThinLTO parameter.

[mehdi_amini]

Rebase on trunk

[tejohnson]

lib/Transforms/Utils/FunctionImportUtils.cpp

227

Everything looks good except that this one still needs to be addressed.

[mehdi_amini]

Sorry missed your earlier comment...
We can totally remove "NewExportedValues" as the responsibility is now
on the caller to specify the full list of symbols to promote.

[tejohnson]

In D18122#376888, @joker.eph wrote:

Sorry missed your earlier comment...
We can totally remove "NewExportedValues" as the responsibility is now
on the caller to specify the full list of symbols to promote.

Hmm yes, I think this is left over from when we did promotion on the exporting side via the module linker.

Thanks for the cleanup. Lgtm with a couple of nits.

tools/llvm-link/llvm-link.cpp

200

Index declaration can be sunk inside if body now

270

This can be moved inside of if statement here too.

[mehdi_amini]

Update before commit

[mehdi_amini]

Thanks!
Committed r263863

include/llvm/Linker/Linker.h

 //===- Linker.h - Module Linker Interface -----------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_LINKER_LINKER_H
 #define LLVM_LINKER_LINKER_H
 
-#include "llvm/IR/ModuleSummaryIndex.h"
 #include "llvm/Linker/IRMover.h"
 
 namespace llvm {
 class Module;
 class StructType;
 class Type;
 
 /// This class provides the core functionality of linking in LLVM. It keeps a
public:
 
   Linker(Module &M);
 
   /// \brief Link \p Src into the composite.
   ///
   /// Passing OverrideSymbols as true will have symbols from Src
   /// shadow those in the Dest.
   /// For ThinLTO function importing/exporting the \p ModuleSummaryIndex
-  /// is passed. If \p FunctionsToImport is provided, only the functions that
+  /// is passed. If \p GlobalsToImport is provided, only the globals that
   /// are part of the set will be imported from the source module.
   /// The \p ValIDToTempMDMap is populated by the linker when function
   /// importing is performed.
   ///
   /// Returns true on error.
   bool linkInModule(std::unique_ptr<Module> Src, unsigned Flags = Flags::None,
-                    const ModuleSummaryIndex *Index = nullptr,
-                    DenseSet<const GlobalValue *> *FunctionsToImport = nullptr,
+                    DenseSet<const GlobalValue *> *GlobalsToImport = nullptr,
                     DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr);
 
   static bool linkModules(Module &Dest, std::unique_ptr<Module> Src,
                           unsigned Flags = Flags::None);
 
   /// \brief Link metadata from \p Src into the composite.
   ///
   /// The \p ValIDToTempMDMap sound have been populated earlier during function
   /// importing from \p Src.
   bool linkInMetadata(std::unique_ptr<Module> Src,
                       DenseMap<unsigned, MDNode *> *ValIDToTempMDMap);
 };
 
 } // End llvm namespace
 
 #endif

include/llvm/Transforms/Utils/FunctionImportUtils.h

 /// function importing, including linkage changes and any necessary renaming.
 class FunctionImportGlobalProcessing {
   /// The Module which we are exporting or importing functions from.
   Module &M;
 
   /// Module summary index passed in for function importing/exporting handling.
   const ModuleSummaryIndex &ImportIndex;
 
-  /// Functions to import from this module, all other functions will be
+  /// Globals to import from this module, all other functions will be
   /// imported as declarations instead of definitions.
-  DenseSet<const GlobalValue *> *FunctionsToImport;
+  DenseSet<const GlobalValue *> *GlobalsToImport;
 
   /// Set to true if the given ModuleSummaryIndex 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 = false;
 
   /// Populated during ThinLTO global processing with locals promoted
   /// to global scope in an exporting module, which now need to be linked
   /// in if calling from the ModuleLinker.
   SetVector<GlobalValue *> NewExportedValues;
 
   /// Check if we should promote the given local value to global scope.
   bool doPromoteLocalToGlobal(const GlobalValue *SGV);
 
   /// Helper methods to check if we are importing from or potentially
   /// exporting from the current source module.
-  bool isPerformingImport() const { return FunctionsToImport != nullptr; }
+  bool isPerformingImport() const { return GlobalsToImport != nullptr; }
   bool isModuleExporting() const { return HasExportedFunctions; }
 
   /// If we are importing from the source module, checks if we should
   /// import SGV as a definition, otherwise import as a declaration.
   bool doImportAsDefinition(const GlobalValue *SGV);
 
   /// Get the name for SGV that should be used in the linked destination
   /// module. Specifically, this handles the case where we need to rename
class FunctionImportGlobalProcessing {
   /// Get the new linkage for SGV that should be used in the linked destination
   /// module. Specifically, for ThinLTO importing or exporting it may need
   /// to be adjusted.
   GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV);
 
 public:
   FunctionImportGlobalProcessing(
       Module &M, const ModuleSummaryIndex &Index,
-      DenseSet<const GlobalValue *> *FunctionsToImport = nullptr)
-      : M(M), ImportIndex(Index), FunctionsToImport(FunctionsToImport) {
+      DenseSet<const GlobalValue *> *GlobalsToImport = nullptr)
+      : M(M), ImportIndex(Index), GlobalsToImport(GlobalsToImport) {
     // If we have a ModuleSummaryIndex but no function to import,
     // then this is the primary module being compiled in a ThinLTO
     // backend compilation, and we need to see if it has functions that
     // may be exported to another backend compilation.
-    if (!FunctionsToImport)
+    if (!GlobalsToImport)
       HasExportedFunctions = ImportIndex.hasExportedFunctions(M);
   }
 
   bool run();
 
   static bool
   doImportAsDefinition(const GlobalValue *SGV,
-                       DenseSet<const GlobalValue *> *FunctionsToImport);
+                       DenseSet<const GlobalValue *> *GlobalsToImport);
 
   /// Access the promoted globals that are now exported and need to be linked.
   SetVector<GlobalValue *> &getNewExportedValues() { return NewExportedValues; }
 };
 
 /// Perform in-place global value handling on the given Module for
 /// exported local functions renamed and promoted for ThinLTO.
-bool renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index);
+bool renameModuleForThinLTO(
+    Module &M, const ModuleSummaryIndex &Index,
+    DenseSet<const GlobalValue *> *GlobalsToImport = nullptr);
 
 } // End llvm namespace
 
 #endif

lib/Linker/LinkModules.cpp

class ModuleLinker {
   std::unique_ptr<Module> SrcM;
 
   SetVector<GlobalValue *> ValuesToLink;
   StringSet<> Internalize;
 
   /// For symbol clashes, prefer those from Src.
   unsigned Flags;
 
-  /// Module summary index passed into ModuleLinker for using in function
-  /// importing/exporting handling.
-  const ModuleSummaryIndex *ImportIndex;
-
   /// Functions to import from source module, all other functions are
   /// imported as declarations instead of definitions.
-  DenseSet<const GlobalValue *> *FunctionsToImport;
-
-  /// Set to true if the given ModuleSummaryIndex 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 = false;
+  DenseSet<const GlobalValue *> *GlobalsToImport;
 
   /// Association between metadata value id and temporary metadata that
   /// remains unmapped after function importing. Saved during function
   /// importing and consumed during the metadata linking postpass.
   DenseMap<unsigned, MDNode *> *ValIDToTempMDMap;
 
   /// Used as the callback for lazy linking.
   /// The mover has just hit GV and we have to decide if it, and other members
GlobalValue *getLinkedToGlobal(const GlobalValue *SrcGV) {
     // Otherwise, we do in fact link to the destination global.
     return DGV;
   }
 
   bool linkIfNeeded(GlobalValue &GV);
 
   /// Helper method to check if we are importing from the current source
   /// module.
-  bool isPerformingImport() const { return FunctionsToImport != nullptr; }
+  bool isPerformingImport() const { return GlobalsToImport != nullptr; }
 
   /// If we are importing from the source module, checks if we should
   /// import SGV as a definition, otherwise import as a declaration.
   bool doImportAsDefinition(const GlobalValue *SGV);
 
 public:
   ModuleLinker(IRMover &Mover, std::unique_ptr<Module> SrcM, unsigned Flags,
-               const ModuleSummaryIndex *Index = nullptr,
-               DenseSet<const GlobalValue *> *FunctionsToImport = nullptr,
+               DenseSet<const GlobalValue *> *GlobalsToImport = nullptr,
                DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr)
-      : Mover(Mover), SrcM(std::move(SrcM)), Flags(Flags), ImportIndex(Index),
-        FunctionsToImport(FunctionsToImport),
-        ValIDToTempMDMap(ValIDToTempMDMap) {
-    assert((ImportIndex || !FunctionsToImport) &&
-           "Expect a ModuleSummaryIndex when importing");
-    // If we have a ModuleSummaryIndex but no function to import,
-    // then this is the primary module being compiled in a ThinLTO
-    // backend compilation, and we need to see if it has functions that
-    // may be exported to another backend compilation.
-    if (ImportIndex && !FunctionsToImport)
-      HasExportedFunctions = ImportIndex->hasExportedFunctions(*this->SrcM);
-  }
+      : Mover(Mover), SrcM(std::move(SrcM)), Flags(Flags),
+        GlobalsToImport(GlobalsToImport), ValIDToTempMDMap(ValIDToTempMDMap) {}
 
   bool run();
 };
 }
 
 bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
   if (!isPerformingImport())
     return false;
-  return FunctionImportGlobalProcessing::doImportAsDefinition(
-      SGV, FunctionsToImport);
+  return FunctionImportGlobalProcessing::doImportAsDefinition(SGV,
+                                                              GlobalsToImport);
 }
 
 static GlobalValue::VisibilityTypes
 getMinVisibility(GlobalValue::VisibilityTypes A,
                  GlobalValue::VisibilityTypes B) {
   if (A == GlobalValue::HiddenVisibility || B == GlobalValue::HiddenVisibility)
     return GlobalValue::HiddenVisibility;
   if (A == GlobalValue::ProtectedVisibility ||
bool ModuleLinker::shouldLinkFromSource(bool &LinkFromSrc,
 
   bool SrcIsDeclaration = Src.isDeclarationForLinker();
   bool DestIsDeclaration = Dest.isDeclarationForLinker();
 
   if (isPerformingImport()) {
     if (isa<Function>(&Src)) {
       // For functions, LinkFromSrc iff this is a function requested
       // for importing. For variables, decide below normally.
-      LinkFromSrc = FunctionsToImport->count(&Src);
+      LinkFromSrc = GlobalsToImport->count(&Src);
       return false;
     }
 
     // Check if this is an alias with an already existing definition
     // in Dest, which must have come from a prior importing pass from
     // the same Src module. Unlike imported function and variable
     // definitions, which are imported as available_externally and are
     // not definitions for the linker, that is not a valid linkage for
bool ModuleLinker::linkIfNeeded(GlobalValue &GV) {
   }
 
   // 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))
+  if (isPerformingImport()) {
+    if (!doImportAsDefinition(&GV))
       return false;
-
-  if (!DGV && !shouldOverrideFromSrc() &&
+  } else if (!DGV && !shouldOverrideFromSrc() &&
              (GV.hasLocalLinkage() || GV.hasLinkOnceLinkage() ||
               GV.hasAvailableExternallyLinkage()))
     return false;
 
   if (GV.isDeclaration())
     return false;
 
   if (const Comdat *SC = GV.getComdat()) {
     bool LinkFromSrc;
     Comdat::SelectionKind SK;
bool ModuleLinker::run() {
   for (Function &SF : *SrcM)
     if (linkIfNeeded(SF))
       return true;
 
   for (GlobalAlias &GA : SrcM->aliases())
     if (linkIfNeeded(GA))
       return true;
 
-  if (ImportIndex) {
-    FunctionImportGlobalProcessing ThinLTOProcessing(*SrcM, *ImportIndex,
-                                                     FunctionsToImport);
-    if (ThinLTOProcessing.run())
-      return true;
-    for (auto *GV : ThinLTOProcessing.getNewExportedValues())
-      ValuesToLink.insert(GV);
-  }
-
   for (unsigned I = 0; I < ValuesToLink.size(); ++I) {
     GlobalValue *GV = ValuesToLink[I];
     const Comdat *SC = GV->getComdat();
     if (!SC)
       continue;
     for (GlobalValue *GV2 : ComdatMembers[SC])
       ValuesToLink.insert(GV2);
   }
bool ModuleLinker::run() {
   }
 
   return false;
 }
 
 Linker::Linker(Module &M) : Mover(M) {}
 
 bool Linker::linkInModule(std::unique_ptr<Module> Src, unsigned Flags,
-                          const ModuleSummaryIndex *Index,
-                          DenseSet<const GlobalValue *> *FunctionsToImport,
+                          DenseSet<const GlobalValue *> *GlobalsToImport,
                           DenseMap<unsigned, MDNode *> *ValIDToTempMDMap) {
-  ModuleLinker ModLinker(Mover, std::move(Src), Flags, Index, FunctionsToImport,
+  ModuleLinker ModLinker(Mover, std::move(Src), Flags, GlobalsToImport,
                          ValIDToTempMDMap);
   return ModLinker.run();
 }
 
 bool Linker::linkInMetadata(std::unique_ptr<Module> Src,
                             DenseMap<unsigned, MDNode *> *ValIDToTempMDMap) {
   SetVector<GlobalValue *> ValuesToLink;
   if (Mover.move(

lib/Transforms/IPO/FunctionImport.cpp

if (!SGV) {
                          "' in Module '" + SrcModule.getModuleIdentifier() +
                          "', error in the summary?\n");
     }
 
     Function *F = dyn_cast<Function>(SGV);
     if (!F && isa<GlobalAlias>(SGV)) {
       auto *SGA = dyn_cast<GlobalAlias>(SGV);
       F = dyn_cast<Function>(SGA->getBaseObject());
-      CalledFunctionName = F->getName();
     }
     assert(F && "Imported Function is ... not a Function");
 
     // We cannot import weak_any functions/aliases without possibly affecting
     // the order they are seen and selected by the linker, changing program
     // semantics.
     if (SGV->hasWeakAnyLinkage()) {
       DEBUG(dbgs() << DestModule.getModuleIdentifier()
                    << ": Ignoring import request for weak-any "
                    << (isa<Function>(SGV) ? "function " : "alias ")
                    << CalledFunctionName << " from "
                    << SrcModule.getModuleIdentifier() << "\n");
       continue;
     }
 
     // Add the function to the import list
     auto &Entry = ModuleToFunctionsToImportMap[SrcModule.getModuleIdentifier()];
     Entry.insert(F);
 
     // Process the newly imported functions and add callees to the worklist.
     // Adjust the threshold
     Threshold = Threshold * ImportInstrFactor;
     F->materialize();
     findExternalCalls(DestModule, *F, Index, VisitedFunctions, Threshold,
                       Worklist);
   }
 }
assert(&DestModule.getContext() == &SrcModule->getContext() &&
            "Context mismatch");
 
     // If modules were created with lazy metadata loading, materialize it
     // now, before linking it (otherwise this will be a noop).
     SrcModule->materializeMetadata();
     UpgradeDebugInfo(*SrcModule);
 
     // Link in the specified functions.
+    if (renameModuleForThinLTO(*SrcModule, Index, &FunctionsToImport))
+      return true;
+
     if (TheLinker.linkInModule(std::move(SrcModule), Linker::Flags::None,
-                               &Index, &FunctionsToImport))
+                               &FunctionsToImport))
       report_fatal_error("Function Import: link error");
 
     ImportedCount += FunctionsToImport.size();
   }
 
   DEBUG(dbgs() << "Imported " << ImportedCount << " functions for Module "
                << DestModule.getModuleIdentifier() << "\n");
   return ImportedCount;
if (!SummaryFile.empty()) {
                << "\n";
         return false;
       }
       Index = IndexPtr.get();
     }
 
     // First we need to promote to global scope and rename any local values that
     // are potentially exported to other modules.
     if (renameModuleForThinLTO(M, *Index)) {
       errs() << "Error renaming module\n";
       return false;
     }
 
     // Perform the import now.
     auto ModuleLoader = [&M](StringRef Identifier) {
       return loadFile(Identifier, M.getContext());
     };

lib/Transforms/Utils/FunctionImportUtils.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/FunctionImportUtils.h"
 using namespace llvm;
 
 /// Checks if we should import SGV as a definition, otherwise import as a
 /// declaration.
 bool FunctionImportGlobalProcessing::doImportAsDefinition(
-    const GlobalValue *SGV, DenseSet<const GlobalValue *> *FunctionsToImport) {
-  auto *GA = dyn_cast<GlobalAlias>(SGV);
-  if (GA) {
+    const GlobalValue *SGV, DenseSet<const GlobalValue *> *GlobalsToImport) {
+
+  // For alias, we tie the definition to the base object. Extract it and recurse
+  if (auto *GA = dyn_cast<GlobalAlias>(SGV)) {
     if (GA->hasWeakAnyLinkage())
       return false;
     const GlobalObject *GO = GA->getBaseObject();
     if (!GO->hasLinkOnceODRLinkage())
       return false;
     return FunctionImportGlobalProcessing::doImportAsDefinition(
-        GO, FunctionsToImport);
+        GO, GlobalsToImport);
   }
-  // Always import GlobalVariable definitions, except for the special
-  // case of WeakAny which are imported as ExternalWeak declarations
-  // (see comments in FunctionImportGlobalProcessing::getLinkage). The linkage
-  // changes described in FunctionImportGlobalProcessing::getLinkage ensure the
-  // correct behavior (e.g. global variables with external linkage are
-  // transformed to available_externally definitions, which are ultimately
-  // turned into declarations after the EliminateAvailableExternally pass).
-  if (isa<GlobalVariable>(SGV) && !SGV->isDeclaration() &&
-      !SGV->hasWeakAnyLinkage())
-    return true;
-  // Only import the function requested for importing.
-  auto *SF = dyn_cast<Function>(SGV);
-  if (SF && FunctionsToImport->count(SF))
+  // Only import the globals requested for importing.
+  if (GlobalsToImport->count(SGV))
     return true;
   // Otherwise no.
   return false;
 }
 
 bool FunctionImportGlobalProcessing::doImportAsDefinition(
     const GlobalValue *SGV) {
   if (!isPerformingImport())
     return false;
-  return FunctionImportGlobalProcessing::doImportAsDefinition(
-      SGV, FunctionsToImport);
+  return FunctionImportGlobalProcessing::doImportAsDefinition(SGV,
+                                                              GlobalsToImport);
 }
 
 bool FunctionImportGlobalProcessing::doPromoteLocalToGlobal(
     const GlobalValue *SGV) {
   assert(SGV->hasLocalLinkage());
   // Both the imported references and the original local variable must
   // be promoted.
   if (!isPerformingImport() && !isModuleExporting())
for (GlobalAlias &GA : M.aliases())
     processGlobalForThinLTO(GA);
 }
 
 bool FunctionImportGlobalProcessing::run() {
   processGlobalsForThinLTO();
   return false;
 }
 
-bool llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index) {
-  FunctionImportGlobalProcessing ThinLTOProcessing(M, Index);
+bool llvm::renameModuleForThinLTO(Module &M, const ModuleSummaryIndex &Index,
+                                  DenseSet<const GlobalValue *> *GlobalsToImport) {
+  FunctionImportGlobalProcessing ThinLTOProcessing(M, Index, GlobalsToImport);
   return ThinLTOProcessing.run();
 }

test/Linker/funcimport.ll

 ; available_externally.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=linkoncefunc:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB5
 ; IMPORTGLOB5-DAG: linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
 ; IMPORTGLOB5-DAG: define linkonce_odr void @linkoncefunc()
 
 ; Ensure that imported static variable and function references are correctly
 ; promoted and renamed (including static constant variable).
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencestatics:%t.bc -S | FileCheck %s --check-prefix=IMPORTSTATIC
-; IMPORTSTATIC-DAG: @staticvar.llvm.1 = available_externally hidden global
+; IMPORTSTATIC-DAG: @staticvar.llvm.1 = external hidden global
 ; IMPORTSTATIC-DAG: @staticconstvar.llvm.1 = internal unnamed_addr constant
 ; IMPORTSTATIC-DAG: define available_externally i32 @referencestatics
 ; IMPORTSTATIC-DAG: %call = call i32 @staticfunc.llvm.1
 ; IMPORTSTATIC-DAG: %0 = load i32, i32* @staticvar.llvm.1
 ; IMPORTSTATIC-DAG: declare hidden i32 @staticfunc.llvm.1
 
 ; Ensure that imported global (external) function and variable references
 ; are handled correctly (including referenced variable imported as
 ; available_externally definition)
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referenceglobals:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOBALS
-; IMPORTGLOBALS-DAG: @globalvar = available_externally global
+; IMPORTGLOBALS-DAG: @globalvar = external global
 ; IMPORTGLOBALS-DAG: declare void @globalfunc1()
 ; IMPORTGLOBALS-DAG: define available_externally i32 @referenceglobals
 
 ; Ensure that common variable correctly imported as common defition.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=referencecommon:%t.bc -S | FileCheck %s --check-prefix=IMPORTCOMMON
-; IMPORTCOMMON-DAG: @commonvar = common global
+; IMPORTCOMMON-DAG: @commonvar = external global
 ; IMPORTCOMMON-DAG: define available_externally i32 @referencecommon
 
 ; Ensure that imported static function pointer correctly promoted and renamed.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=callfuncptr:%t.bc -S | FileCheck %s --check-prefix=IMPORTFUNCPTR
-; IMPORTFUNCPTR-DAG: @P.llvm.1 = available_externally hidden global void ()* null
+; IMPORTFUNCPTR-DAG: @P.llvm.1 = external hidden global void ()*
 ; IMPORTFUNCPTR-DAG: define available_externally void @callfuncptr
 ; IMPORTFUNCPTR-DAG: %0 = load void ()*, void ()** @P.llvm.1
 
 ; Ensure that imported weak function reference/definition handled properly.
 ; Imported weak_any definition should be skipped with warning, and imported
 ; reference should turned into an external_weak declaration.
 ; RUN: llvm-link %t2.bc -summary-index=%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

test/Transforms/FunctionImport/funcimport.ll

 ; CHECK-DAG: define available_externally void @setfuncptr()
 declare void @setfuncptr(...) #1
 
 ; CHECK-DAG: define available_externally void @callfuncptr()
 declare void @callfuncptr(...) #1
 
 ; Ensure that all uses of local variable @P which has used in setfuncptr
 ; and callfuncptr are to the same promoted/renamed global.
-; CHECK-DAG: @P.llvm.2 = available_externally hidden global void ()* null
+; CHECK-DAG: @P.llvm.2 = external hidden global void ()*
 ; CHECK-DAG: %0 = load void ()*, void ()** @P.llvm.2,
 ; CHECK-DAG: store void ()* @staticfunc2.llvm.2, void ()** @P.llvm.2,
 
 ; Won't import weak func
 ; CHECK-DAG: declare void @weakfunc(...)
 declare void @weakfunc(...) #1
 
 ; INSTLIMDEF-DAG: define available_externally hidden void @funcwithpersonality.llvm.2() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 ; INSTLIM5-DAG: declare hidden void @funcwithpersonality.llvm.2()

tools/llvm-link/llvm-link.cpp

 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/SystemUtils.h"
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/Transforms/Utils/FunctionImportUtils.h"
+
 #include <memory>
 using namespace llvm;
 
 static cl::list<std::string>
 InputFilenames(cl::Positional, cl::OneOrMore,
                cl::desc("<input bitcode files>"));
 
 static cl::list<std::string> OverridingInputs(
if (F->hasWeakAnyLinkage()) {
       errs() << "Ignoring import request for weak-any function " << FunctionName
              << " from " << FileName << "\n";
       continue;
     }
 
     if (Verbose)
       errs() << "Importing " << FunctionName << " from " << FileName << "\n";
 
+    // Link in the specified function.
+    DenseSet<const GlobalValue *> GlobalsToImport;
+    GlobalsToImport.insert(F);
+
     std::unique_ptr<ModuleSummaryIndex> Index;
     if (!SummaryIndex.empty()) {
       ErrorOr<std::unique_ptr<ModuleSummaryIndex>> IndexOrErr =
           llvm::getModuleSummaryIndexForFile(SummaryIndex, diagnosticHandler);
       std::error_code EC = IndexOrErr.getError();
       if (EC) {
         errs() << EC.message() << '\n';
         return false;
       }
       Index = std::move(IndexOrErr.get());
+
+      // Linkage Promotion and renaming
+      if (renameModuleForThinLTO(*M, *Index, &GlobalsToImport))
+        return true;
     }
 
     // Save the mapping of value ids to temporary metadata created when
     // importing this function. If we have already imported from this module,
     // add new temporary metadata to the existing mapping.
     auto &TempMDVals = ModuleToTempMDValsMap[FileName];
     if (!TempMDVals)
       TempMDVals = llvm::make_unique<DenseMap<unsigned, MDNode *>>();
 
-    // Link in the specified function.
-    DenseSet<const GlobalValue *> FunctionsToImport;
-    FunctionsToImport.insert(F);
-    if (L.linkInModule(std::move(M), Linker::Flags::None, Index.get(),
-                       &FunctionsToImport, TempMDVals.get()))
+    if (L.linkInModule(std::move(M), Linker::Flags::None, &GlobalsToImport,
+                       TempMDVals.get()))
       return false;
   }
 
   // Now link in metadata for all modules from which we imported functions.
   for (StringMapEntry<std::unique_ptr<DenseMap<unsigned, MDNode *>>> &SME :
        ModuleToTempMDValsMap) {
     // Load the specified source module.
     std::unique_ptr<Module> M = loadFile(argv0, SME.getKey(), Context, true);
for (const auto &File : Files) {
 
     if (verifyModule(*M, &errs())) {
       errs() << argv0 << ": " << File << ": error: input module is broken!\n";
       return false;
     }
 
     // If a module summary index is supplied, load it so linkInModule can treat
     // local functions/variables as exported and promote if necessary.
     std::unique_ptr<ModuleSummaryIndex> Index;
     if (!SummaryIndex.empty()) {
       ErrorOr<std::unique_ptr<ModuleSummaryIndex>> IndexOrErr =
           llvm::getModuleSummaryIndexForFile(SummaryIndex, diagnosticHandler);
       std::error_code EC = IndexOrErr.getError();
       if (EC) {
         errs() << EC.message() << '\n';
         return false;
       }
       Index = std::move(IndexOrErr.get());
+
+      // Promotion
+      if (renameModuleForThinLTO(*M, *Index))
+        return true;
     }
 
     if (Verbose)
       errs() << "Linking in '" << File << "'\n";
 
-    if (L.linkInModule(std::move(M), ApplicableFlags, Index.get()))
+    if (L.linkInModule(std::move(M), ApplicableFlags))
       return false;
     // All linker flags apply to linking of subsequent files.
     ApplicableFlags = Flags;
   }
 
   return true;
 }
 
 int main(int argc, char **argv) {
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal();
   PrettyStackTraceProgram X(argc, argv);