[ThinLTO] Enable in-place symbol changes for exporting module

Summary:
Move ThinLTO global value processing functions out of ModuleLinker and
into a new ThinLTOGlobalProcessor class, which performs any necessary
linkage and naming changes on the given module in place.

As a result, renameModuleForThinLTO no longer needs to create a new
Module when performing any necessary local to global promotion on a
module that we are possibly exporting from during a ThinLTO backend
compilation.

During function importing the ThinLTO processing is still invoked from
the ModuleLinker (via the new class), as it needs to perform renaming and
linkage changes on the source module, e.g. in order to get the correct
renaming during local to global promotion.

Reviewers: joker.eph

Subscribers: davidxl, llvm-commits, joker.eph

Differential Revision: http://reviews.llvm.org/D15696

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@257174 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Linker/Linker.h

@@ -67,8 +67,8 @@ public:
                       DenseMap<unsigned, MDNode *> *ValIDToTempMDMap);
 };
 
-/// Create a new module with exported local functions renamed and promoted
-/// for ThinLTO.
+/// Perform in-place global value handling on the given Module for
+/// exported local functions renamed and promoted for ThinLTO.
 std::unique_ptr<Module> renameModuleForThinLTO(std::unique_ptr<Module> M,
                                                const FunctionInfoIndex *Index);
 

lib/Linker/LinkModules.cpp

@@ -65,9 +65,6 @@ class ModuleLinker {
     return Flags & Linker::InternalizeLinkedSymbols;
   }
 
-  /// Check if we should promote the given local value to global scope.
-  bool doPromoteLocalToGlobal(const GlobalValue *SGV);
-
   bool shouldLinkFromSource(bool &LinkFromSrc, const GlobalValue &Dest,
                             const GlobalValue &Src);
 
@@ -97,11 +94,11 @@ class ModuleLinker {
     Module &DstM = Mover.getModule();
     // If the source has no name it can't link.  If it has local linkage,
     // there is no name match-up going on.
-    if (!SrcGV->hasName() || GlobalValue::isLocalLinkage(getLinkage(SrcGV)))
+    if (!SrcGV->hasName() || GlobalValue::isLocalLinkage(SrcGV->getLinkage()))
       return nullptr;
 
     // Otherwise see if we have a match in the destination module's symtab.
-    GlobalValue *DGV = DstM.getNamedValue(getName(SrcGV));
+    GlobalValue *DGV = DstM.getNamedValue(SrcGV->getName());
     if (!DGV)
       return nullptr;
 
@@ -116,6 +113,64 @@ class ModuleLinker {
 
   bool linkIfNeeded(GlobalValue &GV);
 
+  /// Helper method to check if we are importing from the current source
+  /// module.
+  bool isPerformingImport() const { return FunctionsToImport != 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, Module &SrcM, unsigned Flags,
+               const FunctionInfoIndex *Index = nullptr,
+               DenseSet<const GlobalValue *> *FunctionsToImport = nullptr,
+               DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr)
+      : Mover(Mover), SrcM(SrcM), Flags(Flags), ImportIndex(Index),
+        FunctionsToImport(FunctionsToImport),
+        ValIDToTempMDMap(ValIDToTempMDMap) {
+    assert((ImportIndex || !FunctionsToImport) &&
+           "Expect a FunctionInfoIndex when importing");
+    // If we have a FunctionInfoIndex 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(SrcM);
+    assert((ValIDToTempMDMap || !FunctionsToImport) &&
+           "Function importing must provide a ValIDToTempMDMap");
+  }
+
+  bool run();
+};
+
+/// Class to handle necessary GlobalValue changes required by ThinLTO including
+/// linkage changes and any necessary renaming.
+class ThinLTOGlobalProcessing {
+  /// The Module which we are exporting or importing functions from.
+  Module &M;
+
+  /// Function index passed in for function importing/exporting handling.
+  const FunctionInfoIndex *ImportIndex;
+
+  /// Functions to import from this module, all other functions will be
+  /// imported as declarations instead of definitions.
+  DenseSet<const GlobalValue *> *FunctionsToImport;
+
+  /// 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 = 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; }
@@ -143,32 +198,30 @@ class ModuleLinker {
   GlobalValue::LinkageTypes getLinkage(const GlobalValue *SGV);
 
 public:
-  ModuleLinker(IRMover &Mover, Module &SrcM, unsigned Flags,
-               const FunctionInfoIndex *Index = nullptr,
-               DenseSet<const GlobalValue *> *FunctionsToImport = nullptr,
-               DenseMap<unsigned, MDNode *> *ValIDToTempMDMap = nullptr)
-      : Mover(Mover), SrcM(SrcM), Flags(Flags), ImportIndex(Index),
-        FunctionsToImport(FunctionsToImport),
-        ValIDToTempMDMap(ValIDToTempMDMap) {
-    assert((ImportIndex || !FunctionsToImport) &&
-           "Expect a FunctionInfoIndex when importing");
+  ThinLTOGlobalProcessing(
+      Module &M, const FunctionInfoIndex *Index,
+      DenseSet<const GlobalValue *> *FunctionsToImport = nullptr)
+      : M(M), ImportIndex(Index), FunctionsToImport(FunctionsToImport) {
     // If we have a FunctionInfoIndex 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(SrcM);
-    assert((ValIDToTempMDMap || !FunctionsToImport) &&
-           "Function importing must provide a ValIDToTempMDMap");
+    if (!FunctionsToImport)
+      HasExportedFunctions = ImportIndex->hasExportedFunctions(M);
   }
 
   bool run();
+
+  /// Access the promoted globals that are now exported and need to be linked.
+  SetVector<GlobalValue *> &getNewExportedValues() { return NewExportedValues; }
 };
 }
 
-bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
-  if (!isPerformingImport())
-    return false;
+/// Checks if we should import SGV as a definition, otherwise import as a
+/// declaration.
+static bool
+doImportAsDefinitionImpl(const GlobalValue *SGV,
+                         DenseSet<const GlobalValue *> *FunctionsToImport) {
   auto *GA = dyn_cast<GlobalAlias>(SGV);
   if (GA) {
     if (GA->hasWeakAnyLinkage())
@@ -176,7 +229,7 @@ bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
     const GlobalObject *GO = GA->getBaseObject();
     if (!GO->hasLinkOnceODRLinkage())
       return false;
-    return doImportAsDefinition(GO);
+    return doImportAsDefinitionImpl(GO, FunctionsToImport);
   }
   // Always import GlobalVariable definitions, except for the special
   // case of WeakAny which are imported as ExternalWeak declarations
@@ -196,7 +249,19 @@ bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
   return false;
 }
 
-bool ModuleLinker::doPromoteLocalToGlobal(const GlobalValue *SGV) {
+bool ThinLTOGlobalProcessing::doImportAsDefinition(const GlobalValue *SGV) {
+  if (!isPerformingImport())
+    return false;
+  return doImportAsDefinitionImpl(SGV, FunctionsToImport);
+}
+
+bool ModuleLinker::doImportAsDefinition(const GlobalValue *SGV) {
+  if (!isPerformingImport())
+    return false;
+  return doImportAsDefinitionImpl(SGV, FunctionsToImport);
+}
+
+bool ThinLTOGlobalProcessing::doPromoteLocalToGlobal(const GlobalValue *SGV) {
   assert(SGV->hasLocalLinkage());
   // Both the imported references and the original local variable must
   // be promoted.
@@ -220,7 +285,7 @@ bool ModuleLinker::doPromoteLocalToGlobal(const GlobalValue *SGV) {
   return true;
 }
 
-std::string ModuleLinker::getName(const GlobalValue *SGV) {
+std::string ThinLTOGlobalProcessing::getName(const GlobalValue *SGV) {
   // For locals that must be promoted to global scope, ensure that
   // the promoted name uniquely identifies the copy in the original module,
   // using the ID assigned during combined index creation. When importing,
@@ -234,7 +299,8 @@ std::string ModuleLinker::getName(const GlobalValue *SGV) {
   return SGV->getName();
 }
 
-GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
+GlobalValue::LinkageTypes
+ThinLTOGlobalProcessing::getLinkage(const GlobalValue *SGV) {
   // Any local variable that is referenced by an exported function needs
   // to be promoted to global scope. Since we don't currently know which
   // functions reference which local variables/functions, we must treat
@@ -298,8 +364,7 @@ GlobalValue::LinkageTypes ModuleLinker::getLinkage(const GlobalValue *SGV) {
     // since it would cause global constructors/destructors to be
     // executed multiple times. This should have already been handled
     // 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.
+    // if we try to import, so we simply return AppendingLinkage.
     return GlobalValue::AppendingLinkage;
 
   case GlobalValue::InternalLinkage:
@@ -652,7 +717,7 @@ void ModuleLinker::addLazyFor(GlobalValue &GV, IRMover::ValueAdder Add) {
   }
 }
 
-void ModuleLinker::processGlobalForThinLTO(GlobalValue &GV) {
+void ThinLTOGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) {
   if (GV.hasLocalLinkage() &&
       (doPromoteLocalToGlobal(&GV) || isPerformingImport())) {
     GV.setName(getName(&GV));
@@ -660,21 +725,26 @@ void ModuleLinker::processGlobalForThinLTO(GlobalValue &GV) {
     if (!GV.hasLocalLinkage())
       GV.setVisibility(GlobalValue::HiddenVisibility);
     if (isModuleExporting())
-      ValuesToLink.insert(&GV);
+      NewExportedValues.insert(&GV);
     return;
   }
   GV.setLinkage(getLinkage(&GV));
 }
 
-void ModuleLinker::processGlobalsForThinLTO() {
-  for (GlobalVariable &GV : SrcM.globals())
+void ThinLTOGlobalProcessing::processGlobalsForThinLTO() {
+  for (GlobalVariable &GV : M.globals())
     processGlobalForThinLTO(GV);
-  for (Function &SF : SrcM)
+  for (Function &SF : M)
     processGlobalForThinLTO(SF);
-  for (GlobalAlias &GA : SrcM.aliases())
+  for (GlobalAlias &GA : M.aliases())
     processGlobalForThinLTO(GA);
 }
 
+bool ThinLTOGlobalProcessing::run() {
+  processGlobalsForThinLTO();
+  return false;
+}
+
 bool ModuleLinker::run() {
   for (const auto &SMEC : SrcM.getComdatSymbolTable()) {
     const Comdat &C = SMEC.getValue();
@@ -713,7 +783,14 @@ bool ModuleLinker::run() {
     if (linkIfNeeded(GA))
       return true;
 
-  processGlobalsForThinLTO();
+  if (ImportIndex) {
+    ThinLTOGlobalProcessing 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];
@@ -789,12 +866,10 @@ bool Linker::linkModules(Module &Dest, std::unique_ptr<Module> Src,
 std::unique_ptr<Module>
 llvm::renameModuleForThinLTO(std::unique_ptr<Module> M,
                              const FunctionInfoIndex *Index) {
-  std::unique_ptr<llvm::Module> RenamedModule(
-      new llvm::Module(M->getModuleIdentifier(), M->getContext()));
-  Linker L(*RenamedModule.get());
-  if (L.linkInModule(std::move(M), llvm::Linker::Flags::None, Index))
+  ThinLTOGlobalProcessing ThinLTOProcessing(*M, Index);
+  if (ThinLTOProcessing.run())
     return nullptr;
-  return RenamedModule;
+  return M;
 }
 
 //===----------------------------------------------------------------------===//