ThinLTO: Move the ODR resolution to be based purely on the summary.

This is a requirement for the cache handling in D18494

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

From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 266519

3 files changed, 123 insertions, 85 deletions

diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
index cc1e8a9657c..8ca5e27ea21 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -89,6 +89,19 @@ void ModuleSummaryIndex::collectDefinedFunctionsForModule(
   }
 }
 
+// Collect for each module the list of function it defines (GUID -> Summary).
+void ModuleSummaryIndex::collectDefinedGVSummariesPerModule(
+    StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>> &
+        Module2FunctionInfoMap) const {
+  for (auto &GlobalList : *this) {
+    auto GUID = GlobalList.first;
+    for (auto &GlobInfo : GlobalList.second) {
+      auto *Summary = GlobInfo->summary();
+      Module2FunctionInfoMap[Summary->modulePath()][GUID] = Summary;
+    }
+  }
+}
+
 GlobalValueInfo *
 ModuleSummaryIndex::getGlobalValueInfo(uint64_t ValueGUID,
                                        bool PerModuleIndex) const {
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index c9e91eb2202..ab13ec821aa 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -30,6 +30,7 @@
 #include "llvm/Linker/Linker.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Object/ModuleSummaryIndexObjectFile.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
@@ -42,6 +43,8 @@
 
 using namespace llvm;
 
+#define DEBUG_TYPE "thinlto"
+
 namespace llvm {
 // Flags -discard-value-names, defined in LTOCodeGenerator.cpp
 extern cl::opt<bool> LTODiscardValueNames;
@@ -119,24 +122,15 @@ bool IsFirstDefinitionForLinker(const GlobalValueInfoList &GVInfo,
   return true;
 }
 
-static void ResolveODR(GlobalValue &GV, const ModuleSummaryIndex &Index,
-                             StringRef ModulePath) {
-  if (GV.isDeclaration())
-    return;
-
+static GlobalValue::LinkageTypes ResolveODR(const ModuleSummaryIndex &Index,
+                                            StringRef ModuleIdentifier,
+                                            GlobalValue::GUID GUID,
+                                            const GlobalValueSummary &GV) {
   auto HasMultipleCopies =
       [&](const GlobalValueInfoList &GVInfo) { return GVInfo.size() > 1; };
 
-  auto getGVInfo = [&](GlobalValue &GV) -> const GlobalValueInfoList *{
-    auto GUID = Function::getGlobalIdentifier(GV.getName(), GV.getLinkage(),
-                                              ModulePath);
-    auto It = Index.findGlobalValueInfoList(GV.getName());
-    if (It == Index.end())
-      return nullptr;
-    return &It->second;
-  };
-
-  switch (GV.getLinkage()) {
+  auto OriginalLinkage = GV.linkage();
+  switch (OriginalLinkage) {
   case GlobalValue::ExternalLinkage:
   case GlobalValue::AvailableExternallyLinkage:
   case GlobalValue::AppendingLinkage:
@@ -149,20 +143,19 @@ static void ResolveODR(GlobalValue &GV, const ModuleSummaryIndex &Index,
     break;
   case GlobalValue::LinkOnceODRLinkage:
   case GlobalValue::WeakODRLinkage: {
-    auto *GVInfo = getGVInfo(GV);
-    if (!GVInfo)
-      break;
+    auto &GVInfo = Index.findGlobalValueInfoList(GUID)->second;
     // We need to emit only one of these, the first module will keep
     // it, but turned into a weak while the others will drop it.
-    if (!HasMultipleCopies(*GVInfo))
+    if (!HasMultipleCopies(GVInfo))
       break;
-    if (IsFirstDefinitionForLinker(*GVInfo, Index, ModulePath))
-      GV.setLinkage(GlobalValue::WeakODRLinkage);
+    if (IsFirstDefinitionForLinker(GVInfo, Index, ModuleIdentifier))
+      return GlobalValue::WeakODRLinkage;
     else
-      GV.setLinkage(GlobalValue::AvailableExternallyLinkage);
+      return GlobalValue::AvailableExternallyLinkage;
     break;
   }
   }
+  return OriginalLinkage;
 }
 
 /// Resolve LinkOnceODR and WeakODR.
@@ -171,8 +164,11 @@ static void ResolveODR(GlobalValue &GV, const ModuleSummaryIndex &Index,
 /// current module. However there is a chance that another module is still
 /// referencing them because of the import. We make sure we always emit at least
 /// one copy.
-static void ResolveODR(Module &TheModule,
-                             const ModuleSummaryIndex &Index) {
+static void ResolveODR(
+    const ModuleSummaryIndex &Index,
+    const std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedGlobals,
+    StringRef ModuleIdentifier,
+    DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
   if (Index.modulePaths().size() == 1)
     // Nothing to do if we don't have multiple modules
     return;
@@ -180,20 +176,42 @@ static void ResolveODR(Module &TheModule,
   // We won't optimize the globals that are referenced by an alias for now
   // Ideally we should turn the alias into a global and duplicate the definition
   // when needed.
-  DenseSet<GlobalValue *> GlobalInvolvedWithAlias;
-  for (auto &GA : TheModule.aliases()) {
-    auto *GO = GA.getBaseObject();
-    if (auto *GV = dyn_cast<GlobalValue>(GO))
-      GlobalInvolvedWithAlias.insert(GV);
+  DenseSet<GlobalValueSummary *> GlobalInvolvedWithAlias;
+  for (auto &GA : DefinedGlobals) {
+    if (auto AS = dyn_cast<AliasSummary>(GA.second))
+      GlobalInvolvedWithAlias.insert(&AS->getAliasee());
+  }
+
+  for (auto &GV : DefinedGlobals) {
+    if (GlobalInvolvedWithAlias.count(GV.second))
+      continue;
+    auto NewLinkage = ResolveODR(Index, ModuleIdentifier, GV.first, *GV.second);
+    if (NewLinkage != GV.second->linkage()) {
+      ResolvedODR[GV.first] = NewLinkage;
+    }
   }
+}
+
+/// Fixup linkage, see ResolveODR() above.
+void fixupODR(
+    Module &TheModule,
+    const DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
   // Process functions and global now
   for (auto &GV : TheModule) {
-    if (!GlobalInvolvedWithAlias.count(&GV))
-      ResolveODR(GV, Index, TheModule.getModuleIdentifier());
+    auto NewLinkage = ResolvedODR.find(GV.getGUID());
+    if (NewLinkage == ResolvedODR.end())
+      continue;
+    DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
+                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
+    GV.setLinkage(NewLinkage->second);
   }
   for (auto &GV : TheModule.globals()) {
-    if (!GlobalInvolvedWithAlias.count(&GV))
-      ResolveODR(GV, Index, TheModule.getModuleIdentifier());
+    auto NewLinkage = ResolvedODR.find(GV.getGUID());
+    if (NewLinkage == ResolvedODR.end())
+      continue;
+    DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
+                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
+    GV.setLinkage(NewLinkage->second);
   }
 }
 
@@ -291,13 +309,13 @@ std::unique_ptr<MemoryBuffer> codegenModule(Module &TheModule,
   return make_unique<ObjectMemoryBuffer>(std::move(OutputBuffer));
 }
 
-static std::unique_ptr<MemoryBuffer>
-ProcessThinLTOModule(Module &TheModule, const ModuleSummaryIndex &Index,
-                     StringMap<MemoryBufferRef> &ModuleMap, TargetMachine &TM,
-                     const FunctionImporter::ImportMapTy &ImportList,
-                     ThinLTOCodeGenerator::CachingOptions CacheOptions,
-                     bool DisableCodeGen, StringRef SaveTempsDir,
-                     unsigned count) {
+static std::unique_ptr<MemoryBuffer> ProcessThinLTOModule(
+    Module &TheModule, const ModuleSummaryIndex &Index,
+    StringMap<MemoryBufferRef> &ModuleMap, TargetMachine &TM,
+    const FunctionImporter::ImportMapTy &ImportList,
+    DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
+    ThinLTOCodeGenerator::CachingOptions CacheOptions, bool DisableCodeGen,
+    StringRef SaveTempsDir, unsigned count) {
 
   // Save temps: after IPO.
   saveTempBitcode(TheModule, SaveTempsDir, count, ".1.IPO.bc");
@@ -311,7 +329,7 @@ ProcessThinLTOModule(Module &TheModule, const ModuleSummaryIndex &Index,
     // Resolve the LinkOnce/Weak ODR, trying to turn them into
     // "available_externally" when possible.
     // This is a compile-time optimization.
-    ResolveODR(TheModule, Index);
+    fixupODR(TheModule, ResolvedODR);
 
     // Save temps: after promotion.
     saveTempBitcode(TheModule, SaveTempsDir, count, ".2.promoted.bc");
@@ -435,10 +453,19 @@ std::unique_ptr<ModuleSummaryIndex> ThinLTOCodeGenerator::linkCombinedIndex() {
  */
 void ThinLTOCodeGenerator::promote(Module &TheModule,
                                    ModuleSummaryIndex &Index) {
+  auto ModuleIdentifier = TheModule.getModuleIdentifier();
+  // Collect for each module the list of function it defines (GUID -> Summary).
+  StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
+      ModuleToDefinedGVSummaries;
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Resolve the LinkOnceODR, trying to turn them into "available_externally"
   // where possible.
-  ResolveODR(TheModule, Index);
+  // This is a compile-time optimization.
+  DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
+  ResolveODR(Index, ModuleToDefinedGVSummaries[ModuleIdentifier],
+             ModuleIdentifier, ResolvedODR);
+  fixupODR(TheModule, ResolvedODR);
 
   promoteModule(TheModule, Index);
 }
@@ -449,12 +476,18 @@ void ThinLTOCodeGenerator::promote(Module &TheModule,
 void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
                                              ModuleSummaryIndex &Index) {
   auto ModuleMap = generateModuleMap(Modules);
+  auto ModuleCount = Index.modulePaths().size();
+
+  // Collect for each module the list of function it defines (GUID -> Summary).
+  StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
+      ModuleToDefinedGVSummaries(ModuleCount);
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
 
   // Generate import/export list
-  auto ModuleCount = Index.modulePaths().size();
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
-  ComputeCrossModuleImport(Index, ImportLists, ExportLists);
+  ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
+                           ExportLists);
   auto &ImportList = ImportLists[TheModule.getModuleIdentifier()];
 
   crossImportIntoModule(TheModule, Index, ModuleMap, ImportList);
@@ -522,11 +555,17 @@ void ThinLTOCodeGenerator::run() {
   auto ModuleMap = generateModuleMap(Modules);
   auto ModuleCount = Modules.size();
 
+  // Collect for each module the list of function it defines (GUID -> Summary).
+  StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
+      ModuleToDefinedGVSummaries(ModuleCount);
+  Index->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+
   // Collect the import/export lists for all modules from the call-graph in the
   // combined index.
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
-  ComputeCrossModuleImport(*Index, ImportLists, ExportLists);
+  ComputeCrossModuleImport(*Index, ModuleToDefinedGVSummaries, ImportLists,
+                           ExportLists);
 
   // Parallel optimizer + codegen
   {
@@ -536,6 +575,11 @@ void ThinLTOCodeGenerator::run() {
       Pool.async([&](int count) {
         LLVMContext Context;
         Context.setDiscardValueNames(LTODiscardValueNames);
+        auto ModuleIdentifier = ModuleBuffer.getBufferIdentifier();
+
+        DenseMap<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
+        ResolveODR(*Index, ModuleToDefinedGVSummaries[ModuleIdentifier],
+                   ModuleIdentifier, ResolvedODR);
 
         // Parse module now
         auto TheModule = loadModuleFromBuffer(ModuleBuffer, Context, false);
@@ -545,10 +589,10 @@ void ThinLTOCodeGenerator::run() {
           saveTempBitcode(*TheModule, SaveTempsDir, count, ".0.original.bc");
         }
 
-        auto &ImportList = ImportLists[TheModule->getModuleIdentifier()];
+        auto &ImportList = ImportLists[ModuleIdentifier];
         ProducedBinaries[count] = ProcessThinLTOModule(
             *TheModule, *Index, ModuleMap, *TMBuilder.create(), ImportList,
-            CacheOptions, DisableCodeGen, SaveTempsDir, count);
+            ResolvedODR, CacheOptions, DisableCodeGen, SaveTempsDir, count);
       }, count);
       count++;
     }
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index d6dfe17518b..91e3695c351 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -143,7 +143,7 @@ using EdgeInfo = std::pair<const FunctionSummary *, unsigned /* Threshold */>;
 static void computeImportForFunction(
     const FunctionSummary &Summary, const ModuleSummaryIndex &Index,
     unsigned Threshold,
-    const std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedFunctions,
+    const std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedGVSummaries,
     SmallVectorImpl<EdgeInfo> &Worklist,
     FunctionImporter::ImportMapTy &ImportsForModule,
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
@@ -151,7 +151,7 @@ static void computeImportForFunction(
     auto GUID = Edge.first.getGUID();
     DEBUG(dbgs() << " edge -> " << GUID << " Threshold:" << Threshold << "\n");
 
-    if (DefinedFunctions.count(GUID)) {
+    if (DefinedGVSummaries.count(GUID)) {
       DEBUG(dbgs() << "ignored! Target already in destination module.\n");
       continue;
     }
@@ -212,7 +212,7 @@ static void computeImportForFunction(
 /// as well as the list of "exports", i.e. the list of symbols referenced from
 /// another module (that may require promotion).
 static void ComputeImportForModule(
-    const std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedFunctions,
+    const std::map<GlobalValue::GUID, GlobalValueSummary *> &DefinedGVSummaries,
     const ModuleSummaryIndex &Index,
     FunctionImporter::ImportMapTy &ImportsForModule,
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
@@ -222,14 +222,17 @@ static void ComputeImportForModule(
 
   // Populate the worklist with the import for the functions in the current
   // module
-  for (auto &FuncInfo : DefinedFunctions) {
-    auto *Summary = FuncInfo.second;
+  for (auto &GVInfo : DefinedGVSummaries) {
+    auto *Summary = GVInfo.second;
     if (auto *AS = dyn_cast<AliasSummary>(Summary))
       Summary = &AS->getAliasee();
-    auto *FuncSummary = cast<FunctionSummary>(Summary);
-    DEBUG(dbgs() << "Initalize import for " << FuncInfo.first << "\n");
+    auto *FuncSummary = dyn_cast<FunctionSummary>(Summary);
+    if (!FuncSummary)
+      // Skip import for global variables
+      continue;
+    DEBUG(dbgs() << "Initalize import for " << GVInfo.first << "\n");
     computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
-                             DefinedFunctions, Worklist, ImportsForModule,
+                             DefinedGVSummaries, Worklist, ImportsForModule,
                              ExportLists);
   }
 
@@ -242,7 +245,7 @@ static void ComputeImportForModule(
     // Adjust the threshold
     Threshold = Threshold * ImportInstrFactor;
 
-    computeImportForFunction(*Summary, Index, Threshold, DefinedFunctions,
+    computeImportForFunction(*Summary, Index, Threshold, DefinedGVSummaries,
                              Worklist, ImportsForModule, ExportLists);
   }
 }
@@ -252,38 +255,16 @@ static void ComputeImportForModule(
 /// Compute all the import and export for every module using the Index.
 void llvm::ComputeCrossModuleImport(
     const ModuleSummaryIndex &Index,
+    const StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>> &
+        ModuleToDefinedGVSummaries,
     StringMap<FunctionImporter::ImportMapTy> &ImportLists,
     StringMap<FunctionImporter::ExportSetTy> &ExportLists) {
-  auto ModuleCount = Index.modulePaths().size();
-
-  // Collect for each module the list of function it defines.
-  // GUID -> Summary
-  StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
-      Module2FunctionInfoMap(ModuleCount);
-
-  for (auto &GlobalList : Index) {
-    auto GUID = GlobalList.first;
-    for (auto &GlobInfo : GlobalList.second) {
-      auto *Summary = GlobInfo->summary();
-      if (isa<GlobalVarSummary>(Summary))
-        /// Ignore global variable, focus on functions
-        continue;
-      if (auto *AS = dyn_cast<AliasSummary>(Summary))
-        if (isa<GlobalVarSummary>(&AS->getAliasee()))
-          /// Ignore alias to global variable, focus on functions
-          continue;
-      DEBUG(dbgs() << "Adding definition: Module '" << Summary->modulePath()
-                   << "' defines '" << GUID << "'\n");
-      Module2FunctionInfoMap[Summary->modulePath()][GUID] = Summary;
-    }
-  }
-
   // For each module that has function defined, compute the import/export lists.
-  for (auto &DefinedFunctions : Module2FunctionInfoMap) {
-    auto &ImportsForModule = ImportLists[DefinedFunctions.first()];
-    DEBUG(dbgs() << "Computing import for Module '" << DefinedFunctions.first()
-                 << "'\n");
-    ComputeImportForModule(DefinedFunctions.second, Index, ImportsForModule,
+  for (auto &DefinedGVSummaries : ModuleToDefinedGVSummaries) {
+    auto &ImportsForModule = ImportLists[DefinedGVSummaries.first()];
+    DEBUG(dbgs() << "Computing import for Module '"
+                 << DefinedGVSummaries.first() << "'\n");
+    ComputeImportForModule(DefinedGVSummaries.second, Index, ImportsForModule,
                            &ExportLists);
   }