[ThinLTO] Thin link efficiency: Pre-compute set of non-exportable GVs

[tejohnson]

Summary

Rather than looking up each referenced GUID in the index for each
value we consider importing, in order to see if it is not exportable
(e.g. needs renaming but can't be renamed), pre-compute the set of GUIDs
that can't be exported and consult that.

For a large application with aggressive importing thresholds, the index
lookup findGlobalValueSummaryList in canBeExternallyReferenced was
at the top of the profile. This change reduced the thin link time by
19% on average (the improvement was <1% on average with default
importing thresholds). For my large app, with default thresholds, there
were previously 6.1M index lookups in canBeExternallyReferenced, but with
aggressive importing thresholds there were 195M checks. The index
has 2.8M entries, while the pre-computed DoNotExport set has 13K entries.
There was no significant memory increase from the extra set.

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

144

What about replacing all of this logic by a bit in the summary that would be set during the compile phase?

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

144

I like that idea. I started adding a flag this morning that was specific to this case (HasNotExportableReference). But it occurred to me - should we just subsume all checks done by eligibleToImport into a single flag like NotEligibleToImport? That would subsume the new flag as well as the NoRename, IsNotViableToInline and HasInlineAsmMaybeReferencingInternal flags. I.e. anything that today has any of the NoRename, IsNotViableToInline or HasInlineAsmMaybeReferencingInternal set would be marked NotEligibleToImport. Additionally, anything that references something that would today be marked NoRename would also be marked NotEligibleToImport (the case I want to add here).

With any of these new flags we'd need to bump the INDEX_VERSION. However, with the approach of subsuming all of these flags into one we'd need to conservatively always set NotEligibleToImport for old versions, whereas with a new flag we could reproduce it on the fly for older version bitcode out of the other flags (essentially do what the current eligibleToImport() routine does). Not sure how important it is to optimize for this case (handling the older bitcode) though.

WDYT?

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

144

But it occurred to me - should we just subsume all checks done by eligibleToImport into a single flag like NotEligibleToImport?

That's what I had in mind :)

hat would subsume the new flag as well as the NoRename, IsNotViableToInline and HasInlineAsmMaybeReferencingInternal flags. I.e. anything that today has any of the NoRename, IsNotViableToInline or HasInlineAsmMaybeReferencingInternal set would be marked NotEligibleToImport.

I haven't thought about the exact list of flags that this would subsume, but I trust you to eliminate what's not needed!

Not sure how important it is to optimize for this case (handling the older bitcode) though.

Not important IMO. We don't have a widespread adoption of ThinLTO.

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

144

That's what I had in mind :)

Ok, cool. Wanted to confirm before I started ripping things out too much. =) Will proceed.

[tejohnson]

Subsumed by D28169.

include/llvm/IR/ModuleSummaryIndex.h

public:
   /// Flag that this global value possibly references another value that
   /// can't be renamed.
   void setHasInlineAsmMaybeReferencingInternal() {
     Flags.HasInlineAsmMaybeReferencingInternal = true;
   }
 
   /// Return the list of values referenced by this global value definition.
   ArrayRef<ValueInfo> refs() const { return RefEdgeList; }
+
+  /// Return true if this global value can be externally referenced,
+  /// i.e. it does not need renaming (linkage is not local) or renaming
+  /// is possible (does not have a section for instance).
+  bool canBeExternallyReferenced() const {
+    if (!needsRenaming())
+      return true;
+
+    if (noRename())
+      // Can't externally reference a global that needs renaming if has a
+      // section or is referenced from inline assembly, for example.
+      return false;
+
+    return true;
+  }
 };
 
 /// \brief Alias summary information.
 class AliasSummary : public GlobalValueSummary {
   GlobalValueSummary *AliaseeSummary;
 
 public:
   /// Summary constructors.
public:
   void removeEmptySummaryEntries();
 
   /// Collect for the given module the list of function it defines
   /// (GUID -> Summary).
   void collectDefinedFunctionsForModule(StringRef ModulePath,
                                         GVSummaryMapTy &GVSummaryMap) const;
 
   /// Collect for each module the list of Summaries it defines (GUID ->
-  /// Summary).
+  /// Summary), as well as the set of GUIDs that are not safe to export.
   void collectDefinedGVSummariesPerModule(
-      StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries) const;
+      StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
+      DenseSet<GlobalValue::GUID> &DoNotExport) const;
 };
 
 } // End llvm namespace
 
 #endif

include/llvm/Transforms/IPO/FunctionImport.h

 ///
 /// \p ExportLists contains for each Module the set of globals (GUID) that will
 /// be imported by another module, or referenced by such a function. I.e. this
 /// is the set of globals that need to be promoted/renamed appropriately.
 void ComputeCrossModuleImport(
     const ModuleSummaryIndex &Index,
     const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
     StringMap<FunctionImporter::ImportMapTy> &ImportLists,
-    StringMap<FunctionImporter::ExportSetTy> &ExportLists);
+    StringMap<FunctionImporter::ExportSetTy> &ExportLists,
+    DenseSet<GlobalValue::GUID> &DoNotExport);
 
 /// Compute all the imports for the given module using the Index.
 ///
 /// \p ImportList will be populated with a map that can be passed to
 /// FunctionImporter::importFunctions() above (see description there).
 void ComputeCrossModuleImportForModule(
     StringRef ModulePath, const ModuleSummaryIndex &Index,
-    FunctionImporter::ImportMapTy &ImportList);
+    FunctionImporter::ImportMapTy &ImportList,
+    DenseSet<GlobalValue::GUID> &DoNotExport);
 
 /// Compute the set of summaries needed for a ThinLTO backend compilation of
 /// \p ModulePath.
 //
 /// This includes summaries from that module (in case any global summary based
 /// optimizations were recorded) and from any definitions in other modules that
 /// should be imported.
 //

lib/IR/ModuleSummaryIndex.cpp

for (auto &GlobSummary : GlobalList.second) {
       // Ignore summaries from other modules.
       if (Summary->modulePath() != ModulePath)
         continue;
       GVSummaryMap[GUID] = Summary;
     }
   }
 }
 
-// Collect for each module the list of function it defines (GUID -> Summary).
+// Collect for each module the list of function it defines (GUID -> Summary),
+// as well as the set of GUIDs that are not safe to export.
 void ModuleSummaryIndex::collectDefinedGVSummariesPerModule(
-    StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries) const {
+    StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
+    DenseSet<GlobalValue::GUID> &DoNotExport) const {
   for (auto &GlobalList : *this) {
     auto GUID = GlobalList.first;
     for (auto &Summary : GlobalList.second) {
       ModuleToDefinedGVSummaries[Summary->modulePath()][GUID] = Summary.get();
+      if (!Summary->canBeExternallyReferenced())
+        DoNotExport.insert(GUID);
     }
   }
 }
 
 GlobalValueSummary *
 ModuleSummaryIndex::getGlobalValueSummary(uint64_t ValueGUID,
                                           bool PerModuleIndex) const {
   auto SummaryList = findGlobalValueSummaryList(ValueGUID);
   assert(SummaryList != end() && "GlobalValue not found in index");
   assert((!PerModuleIndex || SummaryList->second.size() == 1) &&
          "Expected a single entry per global value in per-module index");
   auto &Summary = SummaryList->second[0];
   return Summary.get();
 }

lib/LTO/LTO.cpp

Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
 
   if (Conf.CombinedIndexHook && !Conf.CombinedIndexHook(ThinLTO.CombinedIndex))
     return Error::success();
 
   // Collect for each module the list of function it defines (GUID ->
   // Summary).
   StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
       ModuleToDefinedGVSummaries(ThinLTO.ModuleMap.size());
+  DenseSet<GlobalValue::GUID> DoNotExport;
   ThinLTO.CombinedIndex.collectDefinedGVSummariesPerModule(
-      ModuleToDefinedGVSummaries);
+      ModuleToDefinedGVSummaries, DoNotExport);
   // Create entries for any modules that didn't have any GV summaries
   // (either they didn't have any GVs to start with, or we suppressed
   // generation of the summaries because they e.g. had inline assembly
   // uses that couldn't be promoted/renamed on export). This is so
   // InProcessThinBackend::start can still launch a backend thread, which
   // is passed the map of summaries for the module, without any special
   // handling for this case.
   for (auto &Mod : ThinLTO.ModuleMap)
     if (!ModuleToDefinedGVSummaries.count(Mod.first))
       ModuleToDefinedGVSummaries.try_emplace(Mod.first);
 
   StringMap<FunctionImporter::ImportMapTy> ImportLists(
       ThinLTO.ModuleMap.size());
   StringMap<FunctionImporter::ExportSetTy> ExportLists(
       ThinLTO.ModuleMap.size());
   StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
 
   if (Conf.OptLevel > 0) {
     ComputeCrossModuleImport(ThinLTO.CombinedIndex, ModuleToDefinedGVSummaries,
-                             ImportLists, ExportLists);
+                             ImportLists, ExportLists, DoNotExport);
 
     std::set<GlobalValue::GUID> ExportedGUIDs;
     for (auto &Res : GlobalResolutions) {
       if (!Res.second.IRName.empty() &&
           Res.second.Partition == GlobalResolution::External)
         ExportedGUIDs.insert(GlobalValue::getGUID(Res.second.IRName));
     }
 

lib/LTO/ThinLTOCodeGenerator.cpp

  */
 void ThinLTOCodeGenerator::promote(Module &TheModule,
                                    ModuleSummaryIndex &Index) {
   auto ModuleCount = Index.modulePaths().size();
   auto ModuleIdentifier = TheModule.getModuleIdentifier();
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries;
-  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                           DoNotExport);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
-                           ExportLists);
+                           ExportLists, DoNotExport);
 
   // Resolve LinkOnce/Weak symbols.
   StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
   resolveWeakForLinkerInIndex(Index, ResolvedODR);
 
   thinLTOResolveWeakForLinkerModule(
       TheModule, ModuleToDefinedGVSummaries[ModuleIdentifier]);
 

  */
 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<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
-  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                           DoNotExport);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
-                           ExportLists);
+                           ExportLists, DoNotExport);
   auto &ImportList = ImportLists[TheModule.getModuleIdentifier()];
 
   crossImportIntoModule(TheModule, Index, ModuleMap, ImportList);
 }
 
 /**
  * Compute the list of summaries needed for importing into module.
  */
 void ThinLTOCodeGenerator::gatherImportedSummariesForModule(
     StringRef ModulePath, ModuleSummaryIndex &Index,
     std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
   auto ModuleCount = Index.modulePaths().size();
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
-  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                           DoNotExport);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
-                           ExportLists);
+                           ExportLists, DoNotExport);
 
   llvm::gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
                                          ImportLists[ModulePath],
                                          ModuleToSummariesForIndex);
 }
 
 /**
  * Emit the list of files needed for importing into module.
  */
 void ThinLTOCodeGenerator::emitImports(StringRef ModulePath,
                                        StringRef OutputName,
                                        ModuleSummaryIndex &Index) {
   auto ModuleCount = Index.modulePaths().size();
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
-  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                           DoNotExport);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
-                           ExportLists);
+                           ExportLists, DoNotExport);
 
   std::error_code EC;
   if ((EC = EmitImportsFiles(ModulePath, OutputName, ImportLists[ModulePath])))
     report_fatal_error(Twine("Failed to open ") + OutputName +
                        " to save imports lists\n");
 }
 
 /**
  * Perform internalization. Index is updated to reflect linkage changes.
  */
 void ThinLTOCodeGenerator::internalize(Module &TheModule,
                                        ModuleSummaryIndex &Index) {
   initTMBuilder(TMBuilder, Triple(TheModule.getTargetTriple()));
   auto ModuleCount = Index.modulePaths().size();
   auto ModuleIdentifier = TheModule.getModuleIdentifier();
 
   // Convert the preserved symbols set from string to GUID
   auto GUIDPreservedSymbols =
       computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
-  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index.collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                           DoNotExport);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
-                           ExportLists);
+                           ExportLists, DoNotExport);
   auto &ExportList = ExportLists[ModuleIdentifier];
 
   // Be friendly and don't nuke totally the module when the client didn't
   // supply anything to preserve.
   if (ExportList.empty() && GUIDPreservedSymbols.empty())
     return;
 
   // Internalization
void ThinLTOCodeGenerator::run() {
   }
 
   // Prepare the module map.
   auto ModuleMap = generateModuleMap(Modules);
   auto ModuleCount = Modules.size();
 
   // Collect for each module the list of function it defines (GUID -> Summary).
   StringMap<GVSummaryMapTy> ModuleToDefinedGVSummaries(ModuleCount);
-  Index->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries);
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                            DoNotExport);
 
   // 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, ModuleToDefinedGVSummaries, ImportLists,
-                           ExportLists);
+                           ExportLists, DoNotExport);
 
   // Convert the preserved symbols set from string to GUID, this is needed for
   // computing the caching hash and the internalization.
   auto GUIDPreservedSymbols =
       computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
 
   // We use a std::map here to be able to have a defined ordering when
   // producing a hash for the cache entry.

lib/Transforms/IPO/FunctionImport.cpp

if (!Result) {
     report_fatal_error("Abort");
   }
 
   return Result;
 }
 
 namespace {
 
-// Return true if the Summary describes a GlobalValue that can be externally
-// referenced, i.e. it does not need renaming (linkage is not local) or renaming
-// is possible (does not have a section for instance).
-static bool canBeExternallyReferenced(const GlobalValueSummary &Summary) {
-  if (!Summary.needsRenaming())
-    return true;
-
-  if (Summary.noRename())
-    // Can't externally reference a global that needs renaming if has a section
-    // or is referenced from inline assembly, for example.
-    return false;
-
-  return true;
-}
-
-// Return true if \p GUID describes a GlobalValue that can be externally
-// referenced, i.e. it does not need renaming (linkage is not local) or
-// renaming is possible (does not have a section for instance).
-static bool canBeExternallyReferenced(const ModuleSummaryIndex &Index,
-                                      GlobalValue::GUID GUID) {
-  auto Summaries = Index.findGlobalValueSummaryList(GUID);
-  if (Summaries == Index.end())
-    return true;
-  if (Summaries->second.size() != 1)
-    // If there are multiple globals with this GUID, then we know it is
-    // not a local symbol, and it is necessarily externally referenced.
-    return true;
-
-  // We don't need to check for the module path, because if it can't be
-  // externally referenced and we call it, it is necessarilly in the same
-  // module
-  return canBeExternallyReferenced(**Summaries->second.begin());
-}
-
 // Return true if the global described by \p Summary can be imported in another
 // module.
 static bool eligibleForImport(const ModuleSummaryIndex &Index,
-                              const GlobalValueSummary &Summary) {
-  if (!canBeExternallyReferenced(Summary))
+                              const GlobalValueSummary &Summary,
+                              DenseSet<GlobalValue::GUID> &DoNotExport) {
+  if (!Summary.canBeExternallyReferenced())
     // Can't import a global that needs renaming if has a section for instance.
     // FIXME: we may be able to import it by copying it without promotion.
     return false;
 
   // Don't import functions that are not viable to inline.
   if (Summary.isNotViableToInline())
     return false;
 
   // Check references (and potential calls) in the same module. If the current
   // value references a global that can't be externally referenced it is not
   // eligible for import. First check the flag set when we have possible
   // opaque references (e.g. inline asm calls), then check the call and
   // reference sets.
   if (Summary.hasInlineAsmMaybeReferencingInternal())
     return false;
   bool AllRefsCanBeExternallyReferenced =
       llvm::all_of(Summary.refs(), [&](const ValueInfo &VI) {
-        return canBeExternallyReferenced(Index, VI.getGUID());
+        return !DoNotExport.count(VI.getGUID());
       });
   if (!AllRefsCanBeExternallyReferenced)
     return false;
 
   if (auto *FuncSummary = dyn_cast<FunctionSummary>(&Summary)) {
     bool AllCallsCanBeExternallyReferenced = llvm::all_of(
         FuncSummary->calls(), [&](const FunctionSummary::EdgeTy &Edge) {
-          return canBeExternallyReferenced(Index, Edge.first.getGUID());
+          return !DoNotExport.count(Edge.first.getGUID());
         });
     if (!AllCallsCanBeExternallyReferenced)
       return false;
   }
   return true;
 }
 
 /// Given a list of possible callee implementation for a call site, select one
 /// that fits the \p Threshold.
 ///
 /// FIXME: select "best" instead of first that fits. But what is "best"?
 /// - The smallest: more likely to be inlined.
 /// - The one with the least outgoing edges (already well optimized).
 /// - One from a module already being imported from in order to reduce the
 ///   number of source modules parsed/linked.
 /// - One that has PGO data attached.
 /// - [insert you fancy metric here]
 static const GlobalValueSummary *
 selectCallee(const ModuleSummaryIndex &Index,
              const GlobalValueSummaryList &CalleeSummaryList,
-             unsigned Threshold) {
+             unsigned Threshold, DenseSet<GlobalValue::GUID> &DoNotExport) {
   auto It = llvm::find_if(
       CalleeSummaryList,
       [&](const std::unique_ptr<GlobalValueSummary> &SummaryPtr) {
         auto *GVSummary = SummaryPtr.get();
         if (GlobalValue::isInterposableLinkage(GVSummary->linkage()))
           // There is no point in importing these, we can't inline them
           return false;
         if (auto *AS = dyn_cast<AliasSummary>(GVSummary)) {
           GVSummary = &AS->getAliasee();
           // Alias can't point to "available_externally". However when we import
           // linkOnceODR the linkage does not change. So we import the alias
           // and aliasee only in this case.
           // FIXME: we should import alias as available_externally *function*,
           // the destination module does need to know it is an alias.
           if (!GlobalValue::isLinkOnceODRLinkage(GVSummary->linkage()))
             return false;
         }
 
         auto *Summary = cast<FunctionSummary>(GVSummary);
 
         if (Summary->instCount() > Threshold)
           return false;
 
-        if (!eligibleForImport(Index, *Summary))
+        if (!eligibleForImport(Index, *Summary, DoNotExport))
           return false;
 
         return true;
       });
   if (It == CalleeSummaryList.end())
     return nullptr;
 
   return cast<GlobalValueSummary>(It->get());
 }
 
 /// Return the summary for the function \p GUID that fits the \p Threshold, or
 /// null if there's no match.
-static const GlobalValueSummary *selectCallee(GlobalValue::GUID GUID,
-                                              unsigned Threshold,
-                                              const ModuleSummaryIndex &Index) {
+static const GlobalValueSummary *
+selectCallee(GlobalValue::GUID GUID, unsigned Threshold,
+             const ModuleSummaryIndex &Index,
+             DenseSet<GlobalValue::GUID> &DoNotExport) {
   auto CalleeSummaryList = Index.findGlobalValueSummaryList(GUID);
   if (CalleeSummaryList == Index.end())
     return nullptr; // This function does not have a summary
-  return selectCallee(Index, CalleeSummaryList->second, Threshold);
+  return selectCallee(Index, CalleeSummaryList->second, Threshold, DoNotExport);
 }
 
 using EdgeInfo = std::tuple<const FunctionSummary *, unsigned /* Threshold */,
                             GlobalValue::GUID>;
 
 /// Compute the list of functions to import for a given caller. Mark these
 /// imported functions and the symbols they reference in their source module as
 /// exported from their source module.
 static void computeImportForFunction(
     const FunctionSummary &Summary, const ModuleSummaryIndex &Index,
     const unsigned Threshold, const GVSummaryMapTy &DefinedGVSummaries,
     SmallVectorImpl<EdgeInfo> &Worklist,
     FunctionImporter::ImportMapTy &ImportList,
+    DenseSet<GlobalValue::GUID> &DoNotExport,
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
   for (auto &Edge : Summary.calls()) {
     auto GUID = Edge.first.getGUID();
     DEBUG(dbgs() << " edge -> " << GUID << " Threshold:" << Threshold << "\n");
 
     if (DefinedGVSummaries.count(GUID)) {
       DEBUG(dbgs() << "ignored! Target already in destination module.\n");
       continue;
     }
 
     auto GetBonusMultiplier = [](CalleeInfo::HotnessType Hotness) -> float {
       if (Hotness == CalleeInfo::HotnessType::Hot)
         return ImportHotMultiplier;
       if (Hotness == CalleeInfo::HotnessType::Cold)
         return ImportColdMultiplier;
       return 1.0;
     };
 
     const auto NewThreshold =
         Threshold * GetBonusMultiplier(Edge.second.Hotness);
 
-    auto *CalleeSummary = selectCallee(GUID, NewThreshold, Index);
+    auto *CalleeSummary = selectCallee(GUID, NewThreshold, Index, DoNotExport);
     if (!CalleeSummary) {
       DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
       continue;
     }
     // "Resolve" the summary, traversing alias,
     const FunctionSummary *ResolvedCalleeSummary;
     if (isa<AliasSummary>(CalleeSummary)) {
       ResolvedCalleeSummary = cast<FunctionSummary>(

 }
 
 /// Given the list of globals defined in a module, compute the list of imports
 /// 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 GVSummaryMapTy &DefinedGVSummaries, const ModuleSummaryIndex &Index,
     FunctionImporter::ImportMapTy &ImportList,
+    DenseSet<GlobalValue::GUID> &DoNotExport,
     StringMap<FunctionImporter::ExportSetTy> *ExportLists = nullptr) {
   // Worklist contains the list of function imported in this module, for which
   // we will analyse the callees and may import further down the callgraph.
   SmallVector<EdgeInfo, 128> Worklist;
 
   // Populate the worklist with the import for the functions in the current
   // module
   for (auto &GVSummary : DefinedGVSummaries) {
     auto *Summary = GVSummary.second;
     if (auto *AS = dyn_cast<AliasSummary>(Summary))
       Summary = &AS->getAliasee();
     auto *FuncSummary = dyn_cast<FunctionSummary>(Summary);
     if (!FuncSummary)
       // Skip import for global variables
       continue;
     DEBUG(dbgs() << "Initalize import for " << GVSummary.first << "\n");
     computeImportForFunction(*FuncSummary, Index, ImportInstrLimit,
                              DefinedGVSummaries, Worklist, ImportList,
-                             ExportLists);
+                             DoNotExport, ExportLists);
   }
 
   // Process the newly imported functions and add callees to the worklist.
   while (!Worklist.empty()) {
     auto FuncInfo = Worklist.pop_back_val();
     auto *Summary = std::get<0>(FuncInfo);
     auto Threshold = std::get<1>(FuncInfo);
     auto GUID = std::get<2>(FuncInfo);
 
     // Check if we later added this summary with a higher threshold.
     // If so, skip this entry.
     auto ExportModulePath = Summary->modulePath();
     auto &LatestProcessedThreshold = ImportList[ExportModulePath][GUID];
     if (LatestProcessedThreshold > Threshold)
       continue;
 
     computeImportForFunction(*Summary, Index, Threshold, DefinedGVSummaries,
-                             Worklist, ImportList, ExportLists);
+                             Worklist, ImportList, DoNotExport, ExportLists);
   }
 }
 
 } // anonymous namespace
 
 /// Compute all the import and export for every module using the Index.
 void llvm::ComputeCrossModuleImport(
     const ModuleSummaryIndex &Index,
     const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
     StringMap<FunctionImporter::ImportMapTy> &ImportLists,
-    StringMap<FunctionImporter::ExportSetTy> &ExportLists) {
+    StringMap<FunctionImporter::ExportSetTy> &ExportLists,
+    DenseSet<GlobalValue::GUID> &DoNotExport) {
+
   // For each module that has function defined, compute the import/export lists.
   for (auto &DefinedGVSummaries : ModuleToDefinedGVSummaries) {
     auto &ImportList = ImportLists[DefinedGVSummaries.first()];
     DEBUG(dbgs() << "Computing import for Module '"
                  << DefinedGVSummaries.first() << "'\n");
     ComputeImportForModule(DefinedGVSummaries.second, Index, ImportList,
-                           &ExportLists);
+                           DoNotExport, &ExportLists);
   }
 
   // When computing imports we added all GUIDs referenced by anything
   // imported from the module to its ExportList. Now we prune each ExportList
   // of any not defined in that module. This is more efficient than checking
   // while computing imports because some of the summary lists may be long
   // due to linkonce (comdat) copies.
   for (auto &ELI : ExportLists) {
for (auto &ModuleImports : ImportLists) {
     }
   }
 #endif
 }
 
 /// Compute all the imports for the given module in the Index.
 void llvm::ComputeCrossModuleImportForModule(
     StringRef ModulePath, const ModuleSummaryIndex &Index,
-    FunctionImporter::ImportMapTy &ImportList) {
+    FunctionImporter::ImportMapTy &ImportList,
+    DenseSet<GlobalValue::GUID> &DoNotExport) {
 
   // Collect the list of functions this module defines.
   // GUID -> Summary
   GVSummaryMapTy FunctionSummaryMap;
   Index.collectDefinedFunctionsForModule(ModulePath, FunctionSummaryMap);
 
   // Compute the import list for this module.
   DEBUG(dbgs() << "Computing import for Module '" << ModulePath << "'\n");
-  ComputeImportForModule(FunctionSummaryMap, Index, ImportList);
+  ComputeImportForModule(FunctionSummaryMap, Index, ImportList, DoNotExport);
 
 #ifndef NDEBUG
   DEBUG(dbgs() << "* Module " << ModulePath << " imports from "
                << ImportList.size() << " modules.\n");
   for (auto &Src : ImportList) {
     auto SrcModName = Src.first();
     DEBUG(dbgs() << " - " << Src.second.size() << " functions imported from "
                  << SrcModName << "\n");
if (!IndexPtrOrErr) {
     logAllUnhandledErrors(IndexPtrOrErr.takeError(), errs(),
                           "Error loading file '" + SummaryFile + "': ");
     return false;
   }
   std::unique_ptr<ModuleSummaryIndex> Index = std::move(*IndexPtrOrErr);
 
   // First step is collecting the import list.
   FunctionImporter::ImportMapTy ImportList;
-  ComputeCrossModuleImportForModule(M.getModuleIdentifier(), *Index,
-                                    ImportList);
+  StringMap<std::map<GlobalValue::GUID, GlobalValueSummary *>>
+      ModuleToDefinedGVSummaries;
+  DenseSet<GlobalValue::GUID> DoNotExport;
+  Index->collectDefinedGVSummariesPerModule(ModuleToDefinedGVSummaries,
+                                            DoNotExport);
+  ComputeCrossModuleImportForModule(M.getModuleIdentifier(), *Index, ImportList,
+                                    DoNotExport);
 
   // Conservatively mark all internal values as promoted. This interface is
   // only used when doing importing via the function importing pass. The pass
   // is only enabled when testing importing via the 'opt' tool, which does
   // not do the ThinLink that would normally determine what values to promote.
   for (auto &I : *Index) {
     for (auto &S : I.second) {
       if (GlobalValue::isLocalLinkage(S->linkage()))