[ThinLTO] Change ODR resolution and internalization to be index-based

[tejohnson]

Summary

This patch changes the ODR resolution and internalization to be based on
updates to the Index, which are consumed by the backend portion of the
transformations.

It will be followed by an NFC change to move these out of libLTO's
ThinLTOCodeGenerator so that it can be used by other linkers
(gold and lld) and by ThinLTO distributed backends.

The global summary-based portions use callbacks so that the client can
determine the prevailing copy and other information in a client-specific
way. Eventually, with the API being developed in D20268, these may be
modified to use information such as symbol resolutions, supplied by the
clients to the API.

[mehdi_amini]

A few first comments to begin with.

Thanks for doing this, I really planned to do it and you're cleaning behind me actually, wasn't it too painful to follow what was going on? I had the impression that things went a bit out-of-control with time and I was afraid the logic would be hard to refactor.

lib/LTO/LTO.cpp

55 ↗

(On Diff #57365)

s/function/functions/

lib/LTO/ThinLTOCodeGenerator.cpp

366 ↗

(On Diff #57365)

This call disappeared?
I can buy that you don't need it, but I rather have a first "NFC" commit that removes the call here and move the behavior in MustPreserveGV, and then rebase this patch on top of it so the refactoring here would be easier to follow.

793 ↗

(On Diff #57365)

It seems to me that the 30 lines block above should be refactored with the block starting line 533.

It is not clear to me why you need to have this done as a global step, compared to the existing per-thread ResolveODR?

lib/Transforms/IPO/FunctionImport.cpp

536 ↗

(On Diff #57365)

It it not clear if we still need to use the internalize util, is it temporary? What does it do right now other than iterating over the globals and calling the call-back?

[tejohnson]

In D20290#431538, @joker.eph wrote:

A few first comments to begin with.

Thanks for doing this, I really planned to do it and you're cleaning behind me actually, wasn't it too painful to follow what was going on? I had the impression that things went a bit out-of-control with time and I was afraid the logic would be hard to refactor.

No problem, it helps me make forward progress on some other ThinLTO improvements. Wasn't too difficult to follow. Anyway, I reviewed the code, so I had better be able to follow it. =)

lib/LTO/LTO.cpp

55 ↗

(On Diff #57365)

ok

lib/LTO/ThinLTOCodeGenerator.cpp

366 ↗

(On Diff #57365)

We don't need it since the AsmUndefinedRefs is checked by the new MustPreserveGV callback. But the change was actually instigated because UpdateCompilerUsed is in lib/LTO, and I was moving this function to lib/Transforms/IPO, which would have caused a circular dependence.

793 ↗

(On Diff #57365)

Yeah, let me refactor those together.

Needs to be a global step so that distributed builds can use the same approach, communicating the results of this analysis to backends via the index.

lib/Transforms/IPO/FunctionImport.cpp

536 ↗

(On Diff #57365)

Perhaps we could just set the linkage directly for internalized symbols (that's what gold-plugin does for LTO internalization). I was trying to stick closer to the mechanism being used by ThinLTOCodeGenerator.

[mehdi_amini]

lib/Transforms/IPO/FunctionImport.cpp

536 ↗

(On Diff #57365)

Yeah I'm really in favor of moving towards a unified model (i.e. we don't necessarily need the internalize pass, but when I looked at it a few months ago it seemed to me that it was doing some tricky stuff).
That said I'm in favor of moving step-by-step and keeping NFC refactoring separated. For this particular case, if the direction is to get rid of it, a FIXME would seem appropriate to me.

[tejohnson]

lib/Transforms/IPO/FunctionImport.cpp

536 ↗

(On Diff #57365)

Right, it is doing a bunch of checking, so I'd rather not muck with that in this patch. Added a FIXME.

[tejohnson]

• Address comments

[mehdi_amini]

lib/LTO/LTO.cpp

90 ↗

(On Diff #57481)

This does not sound correct to me.

[tejohnson]

lib/LTO/LTO.cpp

90 ↗

(On Diff #57481)

This is needed so that it is not internalized again in thinLTOInternalizeModule (which runs after promotion).

[mehdi_amini]

lib/LTO/LTO.cpp

90 ↗

(On Diff #57481)

I see...
The issue is that this function is called *internalize* and is doing *promotion*.

[tejohnson]

lib/LTO/LTO.cpp

90 ↗

(On Diff #57481)

Good point - how about if I change the name of this and the caller to something like "thinLTOHandleExported(GUID|InIndex)" ?

[mehdi_amini]

lib/LTO/LTO.cpp

90 ↗

(On Diff #57481)

changing the name would be OK to me, whatever you figure is best.
HandleExported does not sound very explicit to me, what about something like resolveLinkage or InternalizeAndPromote?

[tejohnson]

Per IRC discussion, I am going to move the functions back into ThinLTOCodeGenerator.cpp for this patch. It will be followed by an NFC commit to move the functions to the new locations.

lib/LTO/LTO.cpp

90 ↗

(On Diff #57481)

"resolveLinkage" is too general, especially since there is separate handling for linkonce/weak linkages. I like "InternalizeAndPromote". Will make that change.

[tejohnson]

• s/thinLTOInternalize/thinLTOInternalizeAndPromote/(GUID|InIndex)
• Move functions back into ThinLTOCodeGenerator.cpp

[mehdi_amini]

Cannot link llvm-tblgen after this change, I get some strange errors like:

ld: warning: direct access in function 'void std::__1::vector<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo> >::__push_back_slow_path<llvm::CGIOperandList::OperandInfo const&>(llvm::CGIOperandList::OperandInfo const&&&)' from file '/tmp/thinlto.o/21.o' to global weak symbol 'std::__1::__split_buffer<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo>&>::~__split_buffer()' from file 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenInstruction.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

and

ld: internal error: atom not found in symbolIndex(__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcEEEENS_12basic_stringIT_T0_T1_EEPKS6_RKS9_) for architecture x86_64

Something is not NFC here...

[mehdi_amini]

Not sure I totally figured, but here are two things to fix

lib/LTO/ThinLTOCodeGenerator.cpp

387 ↗

(On Diff #57817)

I think one thing is here it should be return Linkage != GlobalValue::InternalLinkage;

(but that's not enough to make the link passing)

539 ↗

(On Diff #57817)

ModuleIdentifier is ignored here, this is causing an issue for thinLTOInternalizeAndPromoteInIndex.

thinLTOInternalizeAndPromoteInIndex should not process the full index but only DefinedGlobals

[tejohnson]

In D20290#435241, @joker.eph wrote:

Cannot link llvm-tblgen after this change, I get some strange errors like:

ld: warning: direct access in function 'void std::__1::vector<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo> >::__push_back_slow_path<llvm::CGIOperandList::OperandInfo const&>(llvm::CGIOperandList::OperandInfo const&&&)' from file '/tmp/thinlto.o/21.o' to global weak symbol 'std::__1::__split_buffer<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo>&>::~__split_buffer()' from file 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenInstruction.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

and

ld: internal error: atom not found in symbolIndex(__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcEEEENS_12basic_stringIT_T0_T1_EEPKS6_RKS9_) for architecture x86_64

Something is not NFC here...

The weak issue seems like it must be related to the linkonce/weak resolution. Let me see if I can repro this here. Meanwhile I will also fix the issues you found, see responses below.

lib/LTO/ThinLTOCodeGenerator.cpp

387 ↗

(On Diff #57817)

Hmm, yes. I think I was assuming that this would only be called for things that are currently external, but that doesn't seem to be the case. Should actually be:

return !GlobalValue::isLocalLinkage(Linkage);

to catch private as well.

539 ↗

(On Diff #57817)

Actually we do want to process the whole index - I can't recall now why I put this here rather than in the caller where it also calls resolveWeakForLinkerInIndex for the entire index. This is bad because multiple threads can be modifying the same index!

[tejohnson]

Fix GV preservation check and move internalization/promotion for global
analysis.

[tejohnson]

In D20290#435534, @tejohnson wrote:

In D20290#435241, @joker.eph wrote:

Cannot link llvm-tblgen after this change, I get some strange errors like:

ld: warning: direct access in function 'void std::__1::vector<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo> >::__push_back_slow_path<llvm::CGIOperandList::OperandInfo const&>(llvm::CGIOperandList::OperandInfo const&&&)' from file '/tmp/thinlto.o/21.o' to global weak symbol 'std::__1::__split_buffer<llvm::CGIOperandList::OperandInfo, std::__1::allocator<llvm::CGIOperandList::OperandInfo>&>::~__split_buffer()' from file 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/CodeGenInstruction.cpp.o' means the weak symbol cannot be overridden at runtime. This was likely caused by different translation units being compiled with different visibility settings.

and

ld: internal error: atom not found in symbolIndex(__ZNSt3__1plIcNS_11char_traitsIcEENS_9allocatorIcEEEENS_12basic_stringIT_T0_T1_EEPKS6_RKS9_) for architecture x86_64

Something is not NFC here...

The weak issue seems like it must be related to the linkonce/weak resolution. Let me see if I can repro this here. Meanwhile I will also fix the issues you found, see responses below.

I couldn't reproduce this locally with gold. I did fix the issues you pointed out. Can you give it a try? If it doesn't fix the issue, could you send me more info about the linkage types we started and ended with for the flagged symbols?

[mehdi_amini]

Can you rebase the patch as well?

lib/LTO/ThinLTOCodeGenerator.cpp

129 ↗

(On Diff #57967)

I hit this assertion, extern template are emitted with available externally apparently.

[mehdi_amini]

Removing the assert, I can link clang

[tejohnson]

In D20290#436122, @joker.eph wrote:

Removing the assert, I can link clang

Great. I assume you have it return null instead? I guess it doesn't matter that we would put null or whatever value in the PrevailingCopies array since it won't be looked at (no copies are weak for linker). Will make that change and rebase.

[tejohnson]

• Remove assert that affected available_externally extern templates in ld64 clang bootstrap. (Rebase coming next)

[tejohnson]

Rebase

[mehdi_amini]

LGTM, with a few minor comments.
(llvm-tblgen is linked bit-to-bit identical with this patch, still building clang for safety)

lib/LTO/ThinLTOCodeGenerator.cpp

337 ↗

(On Diff #58039)

Minor preference for else if

339 ↗

(On Diff #58039)

Can we internalize other than ExternalLinkage?

845 ↗

(On Diff #58039)

The fact that the index is now modified as a side effect should be mentioned in the function description. I'm not sure: can it be reused for internalizing another module?

874 ↗

(On Diff #58039)

Should we move this check a few lines earlier and early return?

962 ↗

(On Diff #58039)

Add in the comment that the *only* reason to have this map is to support the cache.
And I'd also add FIXME: we should be able to compute the caching hash for the entry based on the index, and nuke this map.

[tejohnson]

See question below about linkage types for internalization, will hold off on submit for now.

lib/LTO/ThinLTOCodeGenerator.cpp

337 ↗

(On Diff #58039)

Done.

339 ↗

(On Diff #58039)

Good question. Looking at the rest of the linkage types they are either available externally (skipped by internalizeModule), various types of weak (including linkonce), and other special types such as common and appending.

I'm not sure about the weak/linkonce types - I don't see anything in internalizeModule or elsewhere in HEAD that prevents internalizing these symbols, but I would have thought that internalizing them could be problematic since they have weak linkage and some can be overridden by external definitions (isInterposableLinkage). I assume that with ld64 any that need to be preserved because they are overridden are added to the preserved set? In gold we know which one is prevailing (which is used to guide current internalization decisions in gold-plugin) and can use that info to guide the internalization via the callback when I call this from there.

I removed the "if (GlobalValue::isExternalLinkage(S->linkage()))" guard and the regression tests run fine - do you want to try your bootstrap with that change?

845 ↗

(On Diff #58039)

Done here (and in the description of promote() which modifies the index for resolved weak symbols).

It would be reusable for another module - since the analysis is global on the index the decisions should be the same for all modules. The index updates would happen the first time through here.

874 ↗

(On Diff #58039)

Yes, done.

962 ↗

(On Diff #58039)

Done

[tejohnson]

lib/LTO/ThinLTOCodeGenerator.cpp

339 ↗

(On Diff #58039)

Actually, changed this to "if (!GlobalValue::isLocalLinkage(S->linkage()))" so we don't change private -> internal.

[tejohnson]

• Address comments.

[mehdi_amini]

lib/LTO/ThinLTOCodeGenerator.cpp

339 ↗

(On Diff #58242)

Yes in ld64 and regular monolithic LTO, we are internalizing linkonce_ODR if they are not exported.
Can weak functions defined in the main binary be overriden at runtime?
It may be a platform specific question...

[tejohnson]

lib/LTO/ThinLTOCodeGenerator.cpp

339 ↗

(On Diff #58242)

Agree it is going to be platform specific. So then I think that the new version of the patch is better, and each linker will need to handle appropriately (e.g. here by adding to preserveSymbol(), in gold by using the callback to query gold's resolution, and eventually with pcc's API via the resolution info provided to the API).

Do you want to retest this new version before I commit?

[mehdi_amini]

lib/LTO/ThinLTOCodeGenerator.cpp

339 ↗

(On Diff #58242)

No it should be fine (and I have a bot that will bootstrap clang immediately anyway)

llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h

public:
   /// Get the flags for this GlobalValue (see \p struct GVFlags).
   GVFlags flags() { return Flags; }
 
   /// Return linkage type recorded for this global value.
   GlobalValue::LinkageTypes linkage() const {
     return static_cast<GlobalValue::LinkageTypes>(Flags.Linkage);
   }
 
+  /// Sets the linkage to the value determined by global summary-based
+  /// optimization. Will be applied in the ThinLTO backends.
+  void setLinkage(GlobalValue::LinkageTypes Linkage) {
+    Flags.Linkage = Linkage;
+  }
+
   /// Return true if this summary is for a GlobalValue that needs promotion
   /// to be referenced from another module.
   bool needsRenaming() const { return GlobalValue::isLocalLinkage(linkage()); }
 
   /// Return true if this global value is located in a specific section.
   bool hasSection() const { return Flags.HasSection; }
 
   /// Record a reference from this global value to the global value identified
public:
   /// for the given value name of a local, and its original module's ID.
   static std::string getGlobalNameForLocal(StringRef Name, ModuleHash ModHash) {
     SmallString<256> NewName(Name);
     NewName += ".llvm.";
     NewName += utohexstr(ModHash[0]); // Take the first 32 bits
     return NewName.str();
   }
 
+  /// Helper to obtain the unpromoted name for a global value (or the original
+  /// name if not promoted).
+  static StringRef getOriginalNameBeforePromote(StringRef Name) {
+    std::pair<StringRef, StringRef> Pair = Name.split(".llvm.");
+    return Pair.first;
+  }
+
   /// Add a new module path with the given \p Hash, mapped to the given \p
   /// ModID, and return an iterator to the entry in the index.
   ModulePathStringTableTy::iterator
   addModulePath(StringRef ModPath, uint64_t ModId,
                 ModuleHash Hash = ModuleHash{{0}}) {
     return ModulePathStringTable.insert(std::make_pair(
                                             ModPath,
                                             std::make_pair(ModId, Hash))).first;

llvm/trunk/include/llvm/LTO/ThinLTOCodeGenerator.h

public:
 
   /**
    * Produce the combined summary index from all the bitcode files:
    * "thin-link".
    */
   std::unique_ptr<ModuleSummaryIndex> linkCombinedIndex();
 
   /**
-   * Perform promotion and renaming of exported internal functions.
+   * Perform promotion and renaming of exported internal functions,
+   * and additionally resolve weak and linkonce symbols.
+   * Index is updated to reflect linkage changes from weak resolution.
    */
   void promote(Module &Module, ModuleSummaryIndex &Index);
 
   /**
    * Compute and emit the imported files for module at \p ModulePath.
    */
   static void emitImports(StringRef ModulePath, StringRef OutputName,
                           ModuleSummaryIndex &Index);
 
   /**
    * Perform cross-module importing for the module identified by
    * ModuleIdentifier.
    */
   void crossModuleImport(Module &Module, ModuleSummaryIndex &Index);
 
   /**
    * Compute the list of summaries needed for importing into module.
    */
   static void gatherImportedSummariesForModule(
       StringRef ModulePath, ModuleSummaryIndex &Index,
       std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
 
   /**
-   * Perform internalization.
+   * Perform internalization. Index is updated to reflect linkage changes.
    */
   void internalize(Module &Module, ModuleSummaryIndex &Index);
 
   /**
    * Perform post-importing ThinLTO optimizations.
    */
   void optimize(Module &Module);
 

llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp

 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/ModuleSummaryIndexObjectFile.h"
-#include "llvm/Support/Debug.h"
 #include "llvm/Support/CachePruning.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/IPO.h"
static void saveTempBitcode(const Module &TheModule, StringRef TempDir,
   std::error_code EC;
   raw_fd_ostream OS(SaveTempPath.str(), EC, sys::fs::F_None);
   if (EC)
     report_fatal_error(Twine("Failed to open ") + SaveTempPath +
                        " to save optimized bitcode\n");
   WriteBitcodeToFile(&TheModule, OS, /* ShouldPreserveUseListOrder */ true);
 }
 
-bool IsFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList,
-                                const ModuleSummaryIndex &Index,
-                                StringRef ModulePath) {
+static const GlobalValueSummary *
+getFirstDefinitionForLinker(const GlobalValueSummaryList &GVSummaryList) {
+  // If there is any strong definition anywhere, get it.
+  auto StrongDefForLinker = llvm::find_if(
+      GVSummaryList, [](const std::unique_ptr<GlobalValueSummary> &Summary) {
+        auto Linkage = Summary->linkage();
+        return !GlobalValue::isAvailableExternallyLinkage(Linkage) &&
+               !GlobalValue::isWeakForLinker(Linkage);
+      });
+  if (StrongDefForLinker != GVSummaryList.end())
+    return StrongDefForLinker->get();
   // Get the first *linker visible* definition for this global in the summary
   // list.
   auto FirstDefForLinker = llvm::find_if(
       GVSummaryList, [](const std::unique_ptr<GlobalValueSummary> &Summary) {
         auto Linkage = Summary->linkage();
         return !GlobalValue::isAvailableExternallyLinkage(Linkage);
       });
-  // If \p GV is not the first definition, give up...
-  if ((*FirstDefForLinker)->modulePath() != ModulePath)
-    return false;
-  // If there is any strong definition anywhere, do not bother emitting this.
-  if (llvm::any_of(
-          GVSummaryList,
-          [](const std::unique_ptr<GlobalValueSummary> &Summary) {
-            auto Linkage = Summary->linkage();
-            return !GlobalValue::isAvailableExternallyLinkage(Linkage) &&
-                   !GlobalValue::isWeakForLinker(Linkage);
-          }))
-    return false;
-  return true;
+  // Extern templates can be emitted as available_externally.
+  if (FirstDefForLinker == GVSummaryList.end())
+    return nullptr;
+  return FirstDefForLinker->get();
 }
 
-static GlobalValue::LinkageTypes
-ResolveODR(const ModuleSummaryIndex &Index,
-           const FunctionImporter::ExportSetTy &ExportList,
-           const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
-           StringRef ModuleIdentifier, GlobalValue::GUID GUID,
-           const GlobalValueSummary &GV) {
+// Populate map of GUID to the prevailing copy for any multiply defined
+// symbols. Currently assume first copy is prevailing, or any strong
+// definition. Can be refined with Linker information in the future.
+static void computePrevailingCopies(
+    const ModuleSummaryIndex &Index,
+    DenseMap<GlobalValue::GUID, const GlobalValueSummary *> &PrevailingCopy) {
   auto HasMultipleCopies = [&](const GlobalValueSummaryList &GVSummaryList) {
     return GVSummaryList.size() > 1;
   };
 
-  auto OriginalLinkage = GV.linkage();
-  switch (OriginalLinkage) {
-  case GlobalValue::ExternalLinkage:
-  case GlobalValue::AvailableExternallyLinkage:
-  case GlobalValue::AppendingLinkage:
-  case GlobalValue::InternalLinkage:
-  case GlobalValue::PrivateLinkage:
-  case GlobalValue::ExternalWeakLinkage:
-  case GlobalValue::CommonLinkage:
-  case GlobalValue::LinkOnceAnyLinkage:
-  case GlobalValue::WeakAnyLinkage:
-    break;
-  case GlobalValue::LinkOnceODRLinkage:
-  case GlobalValue::WeakODRLinkage: {
-    auto &GVSummaryList = Index.findGlobalValueSummaryList(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(GVSummaryList)) {
-      // Exported LinkonceODR needs to be promoted to not be discarded
-      if (GlobalValue::isDiscardableIfUnused(OriginalLinkage) &&
-          (ExportList.count(GUID) || GUIDPreservedSymbols.count(GUID)))
-        return GlobalValue::WeakODRLinkage;
-      break;
-    }
-    if (IsFirstDefinitionForLinker(GVSummaryList, Index, ModuleIdentifier))
-      return GlobalValue::WeakODRLinkage;
-    else if (isa<AliasSummary>(&GV))
-      // Alias can't be turned into available_externally.
-      return OriginalLinkage;
-    return GlobalValue::AvailableExternallyLinkage;
+  for (auto &I : Index) {
+    if (HasMultipleCopies(I.second))
+      PrevailingCopy[I.first] = getFirstDefinitionForLinker(I.second);
   }
-  }
-  return OriginalLinkage;
+
+static void thinLTOResolveWeakForLinkerGUID(
+    GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
+    DenseSet<GlobalValueSummary *> &GlobalInvolvedWithAlias,
+    std::function<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+        isPrevailing,
+    std::function<bool(StringRef, GlobalValue::GUID)> isExported,
+    std::function<void(StringRef, GlobalValue::GUID, GlobalValue::LinkageTypes)>
+        recordNewLinkage) {
+  auto HasMultipleCopies = GVSummaryList.size() > 1;
+
+  for (auto &S : GVSummaryList) {
+    if (GlobalInvolvedWithAlias.count(S.get()))
+      continue;
+    GlobalValue::LinkageTypes OriginalLinkage = S->linkage();
+    if (!GlobalValue::isWeakForLinker(OriginalLinkage))
+      continue;
+    // 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 when possible.
+    if (!HasMultipleCopies) {
+      // Exported Linkonce needs to be promoted to not be discarded.
+      // FIXME: This should handle LinkOnceAny as well, but that should be a
+      // follow-on to the NFC restructuring:
+      // if (GlobalValue::isLinkOnceLinkage(OriginalLinkage) &&
+      //     isExported(S->modulePath(), GUID))
+      //   S->setLinkage(GlobalValue::getWeakLinkage(
+      //       GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
+      if (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage) &&
+          isExported(S->modulePath(), GUID))
+        S->setLinkage(GlobalValue::WeakODRLinkage);
+    } else if (isPrevailing(GUID, S.get())) {
+      // FIXME: This should handle LinkOnceAny as well, but that should be a
+      // follow-on to the NFC restructuring:
+      // if (GlobalValue::isLinkOnceLinkage(OriginalLinkage))
+      //   S->setLinkage(GlobalValue::getWeakLinkage(
+      //       GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
+      if (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage))
+        S->setLinkage(GlobalValue::WeakODRLinkage);
+    }
+    // Alias can't be turned into available_externally.
+    else if (!isa<AliasSummary>(S.get()) &&
+             (GlobalValue::isLinkOnceODRLinkage(OriginalLinkage) ||
+              GlobalValue::isWeakODRLinkage(OriginalLinkage)))
+      S->setLinkage(GlobalValue::AvailableExternallyLinkage);
+    if (S->linkage() != OriginalLinkage)
+      recordNewLinkage(S->modulePath(), GUID, S->linkage());
+  }
 }
 
-/// Resolve LinkOnceODR and WeakODR.
-///
-/// We'd like to drop these function if they are no longer referenced in the
-/// 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(
-    const ModuleSummaryIndex &Index,
-    const FunctionImporter::ExportSetTy &ExportList,
-    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
-    const GVSummaryMapTy &DefinedGlobals, StringRef ModuleIdentifier,
-    std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
+// Resolve Weak and LinkOnce values in the \p Index.
+//
+// We'd like to drop these functions if they are no longer referenced in the
+// 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.
+void thinLTOResolveWeakForLinkerInIndex(
+    ModuleSummaryIndex &Index,
+    std::function<bool(GlobalValue::GUID, const GlobalValueSummary *)>
+        isPrevailing,
+    std::function<bool(StringRef, GlobalValue::GUID)> isExported,
+    std::function<void(StringRef, GlobalValue::GUID, GlobalValue::LinkageTypes)>
+        recordNewLinkage) {
   if (Index.modulePaths().size() == 1)
     // Nothing to do if we don't have multiple modules
     return;
 
   // 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<GlobalValueSummary *> GlobalInvolvedWithAlias;
-  for (auto &GA : DefinedGlobals) {
-    if (auto AS = dyn_cast<AliasSummary>(GA.second))
+  for (auto &I : Index)
+    for (auto &S : I.second)
+      if (auto AS = dyn_cast<AliasSummary>(S.get()))
         GlobalInvolvedWithAlias.insert(&AS->getAliasee());
-  }
 
-  for (auto &GV : DefinedGlobals) {
-    if (GlobalInvolvedWithAlias.count(GV.second))
-      continue;
-    auto NewLinkage =
-        ResolveODR(Index, ExportList, GUIDPreservedSymbols, ModuleIdentifier, GV.first, *GV.second);
-    if (NewLinkage != GV.second->linkage()) {
-      ResolvedODR[GV.first] = NewLinkage;
-    }
-  }
+  for (auto &I : Index)
+    thinLTOResolveWeakForLinkerGUID(I.second, I.first, GlobalInvolvedWithAlias,
+                                    isPrevailing, isExported, recordNewLinkage);
 }
 
-/// Fixup linkage, see ResolveODR() above.
-void fixupODR(
-    Module &TheModule,
-    const std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR) {
-  // Process functions and global now
-  for (auto &GV : TheModule) {
-    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()) {
-    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.aliases()) {
-    auto NewLinkage = ResolvedODR.find(GV.getGUID());
-    if (NewLinkage == ResolvedODR.end())
-      continue;
+/// Fixup WeakForLinker linkages in \p TheModule based on summary analysis.
+void thinLTOResolveWeakForLinkerModule(Module &TheModule,
+                                       const GVSummaryMapTy &DefinedGlobals) {
+  auto updateLinkage = [&](GlobalValue &GV) {
+    if (!GlobalValue::isWeakForLinker(GV.getLinkage()))
+      return;
+    // See if the global summary analysis computed a new resolved linkage.
+    const auto &GS = DefinedGlobals.find(GV.getGUID());
+    if (GS == DefinedGlobals.end())
+      return;
+    auto NewLinkage = GS->second->linkage();
+    if (NewLinkage == GV.getLinkage())
+      return;
     DEBUG(dbgs() << "ODR fixing up linkage for `" << GV.getName() << "` from "
-                 << GV.getLinkage() << " to " << NewLinkage->second << "\n");
-    GV.setLinkage(NewLinkage->second);
-  }
+                 << GV.getLinkage() << " to " << NewLinkage << "\n");
+    GV.setLinkage(NewLinkage);
+  };
+
+  // Process functions and global now
+  for (auto &GV : TheModule)
+    updateLinkage(GV);
+  for (auto &GV : TheModule.globals())
+    updateLinkage(GV);
+  for (auto &GV : TheModule.aliases())
+    updateLinkage(GV);
 }
 
 static StringMap<MemoryBufferRef>
 generateModuleMap(const std::vector<MemoryBufferRef> &Modules) {
   StringMap<MemoryBufferRef> ModuleMap;
   for (auto &ModuleBuffer : Modules) {
     assert(ModuleMap.find(ModuleBuffer.getBufferIdentifier()) ==
                ModuleMap.end() &&
static void optimizeModule(Module &TheModule, TargetMachine &TM) {
   PM.add(createTargetTransformInfoWrapperPass(TM.getTargetIRAnalysis()));
 
   // Add optimizations
   PMB.populateThinLTOPassManager(PM);
 
   PM.run(TheModule);
 }
 
-// Create a DenseSet of GlobalValue to be used with the Internalizer.
-static DenseSet<const GlobalValue *> computePreservedSymbolsForModule(
-    Module &TheModule, const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
-    const FunctionImporter::ExportSetTy &ExportList) {
-  DenseSet<const GlobalValue *> PreservedGV;
-  if (GUIDPreservedSymbols.empty())
-    // Early exit: internalize is disabled when there is nothing to preserve.
-    return PreservedGV;
-
-  auto AddPreserveGV = [&](const GlobalValue &GV) {
-    auto GUID = GV.getGUID();
-    if (GUIDPreservedSymbols.count(GUID) || ExportList.count(GUID))
-      PreservedGV.insert(&GV);
-  };
-
-  for (auto &GV : TheModule)
-    AddPreserveGV(GV);
-  for (auto &GV : TheModule.globals())
-    AddPreserveGV(GV);
-  for (auto &GV : TheModule.aliases())
-    AddPreserveGV(GV);
-
-  return PreservedGV;
+static void thinLTOInternalizeAndPromoteGUID(
+    GlobalValueSummaryList &GVSummaryList, GlobalValue::GUID GUID,
+    std::function<bool(StringRef, GlobalValue::GUID)> isExported) {
+  for (auto &S : GVSummaryList) {
+    if (isExported(S->modulePath(), GUID)) {
+      if (GlobalValue::isLocalLinkage(S->linkage()))
+        S->setLinkage(GlobalValue::ExternalLinkage);
+    } else if (!GlobalValue::isLocalLinkage(S->linkage()))
+      S->setLinkage(GlobalValue::InternalLinkage);
   }
-
-// Run internalization on \p TheModule
-static void
-doInternalizeModule(Module &TheModule, const TargetMachine &TM,
-                    const DenseSet<const GlobalValue *> &PreservedGV) {
-  if (PreservedGV.empty()) {
-    // Be friendly and don't nuke totally the module when the client didn't
-    // supply anything to preserve.
-    return;
-  }
 
+// Update the linkages in the given \p Index to mark exported values
+// as external and non-exported values as internal.
+void thinLTOInternalizeAndPromoteInIndex(
+    ModuleSummaryIndex &Index,
+    std::function<bool(StringRef, GlobalValue::GUID)> isExported) {
+  for (auto &I : Index)
+    thinLTOInternalizeAndPromoteGUID(I.second, I.first, isExported);
+}
+
+// Run internalization on \p TheModule based on symmary analysis.
+void thinLTOInternalizeModule(Module &TheModule,
+                              const GVSummaryMapTy &DefinedGlobals) {
   // Parse inline ASM and collect the list of symbols that are not defined in
   // the current module.
   StringSet<> AsmUndefinedRefs;
   object::IRObjectFile::CollectAsmUndefinedRefs(
       Triple(TheModule.getTargetTriple()), TheModule.getModuleInlineAsm(),
       [&AsmUndefinedRefs](StringRef Name, object::BasicSymbolRef::Flags Flags) {
         if (Flags & object::BasicSymbolRef::SF_Undefined)
           AsmUndefinedRefs.insert(Name);
       });
 
-  // Update the llvm.compiler_used globals to force preserving libcalls and
-  // symbols referenced from asm
-  UpdateCompilerUsed(TheModule, TM, AsmUndefinedRefs);
-
   // Declare a callback for the internalize pass that will ask for every
   // candidate GlobalValue if it can be internalized or not.
-  auto MustPreserveGV =
-      [&](const GlobalValue &GV) -> bool { return PreservedGV.count(&GV); };
+  auto MustPreserveGV = [&](const GlobalValue &GV) -> bool {
+    // Can't be internalized if referenced in inline asm.
+    if (AsmUndefinedRefs.count(GV.getName()))
+      return true;
+
+    // Lookup the linkage recorded in the summaries during global analysis.
+    const auto &GS = DefinedGlobals.find(GV.getGUID());
+    GlobalValue::LinkageTypes Linkage;
+    if (GS == DefinedGlobals.end()) {
+      // Must have been promoted (possibly conservatively). Find original
+      // name so that we can access the correct summary and see if it can
+      // be internalized again.
+      // FIXME: Eventually we should control promotion instead of promoting
+      // and internalizing again.
+      StringRef OrigName =
+          ModuleSummaryIndex::getOriginalNameBeforePromote(GV.getName());
+      std::string OrigId = GlobalValue::getGlobalIdentifier(
+          OrigName, GlobalValue::InternalLinkage,
+          TheModule.getSourceFileName());
+      const auto &GS = DefinedGlobals.find(GlobalValue::getGUID(OrigId));
+      assert(GS != DefinedGlobals.end());
+      Linkage = GS->second->linkage();
+    } else
+      Linkage = GS->second->linkage();
+    return !GlobalValue::isLocalLinkage(Linkage);
+  };
 
+  // FIXME: See if we can just internalize directly here via linkage changes
+  // based on the index, rather than invoking internalizeModule.
   llvm::internalizeModule(TheModule, MustPreserveGV);
 }
 
 // Convert the PreservedSymbols map from "Name" based to "GUID" based.
 static DenseSet<GlobalValue::GUID>
 computeGUIDPreservedSymbols(const StringSet<> &PreservedSymbols,
                             const Triple &TheTriple) {
   DenseSet<GlobalValue::GUID> GUIDPreservedSymbols(PreservedSymbols.size());
if (auto EC = ReloadedBufferOrErr.getError()) {
       errs() << "error: can't reload cached file '" << EntryPath
              << "': " << EC.message() << "\n";
       return OutputBuffer;
     }
     return std::move(*ReloadedBufferOrErr);
   }
 };
 
-static std::unique_ptr<MemoryBuffer> ProcessThinLTOModule(
-    Module &TheModule, const ModuleSummaryIndex &Index,
+static std::unique_ptr<MemoryBuffer>
+ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index,
                      StringMap<MemoryBufferRef> &ModuleMap, TargetMachine &TM,
                      const FunctionImporter::ImportMapTy &ImportList,
                      const FunctionImporter::ExportSetTy &ExportList,
                      const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
-    std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> &ResolvedODR,
-    ThinLTOCodeGenerator::CachingOptions CacheOptions, bool DisableCodeGen,
-    StringRef SaveTempsDir, unsigned count) {
-
-  // Prepare for internalization by computing the set of symbols to preserve.
-  // We need to compute the list of symbols to preserve during internalization
-  // before doing any promotion because after renaming we won't (easily) match
-  // to the original name.
-  auto PreservedGV = computePreservedSymbolsForModule(
-      TheModule, GUIDPreservedSymbols, ExportList);
+                     const GVSummaryMapTy &DefinedGlobals,
+                     ThinLTOCodeGenerator::CachingOptions CacheOptions,
+                     bool DisableCodeGen, StringRef SaveTempsDir,
+                     unsigned count) {
 
   // "Benchmark"-like optimization: single-source case
   bool SingleModule = (ModuleMap.size() == 1);
 
   if (!SingleModule) {
     promoteModule(TheModule, Index);
 
-    // Resolve the LinkOnce/Weak ODR, trying to turn them into
-    // "available_externally" when possible.
-    // This is a compile-time optimization.
-    fixupODR(TheModule, ResolvedODR);
+    // Apply summary-based LinkOnce/Weak resolution decisions.
+    thinLTOResolveWeakForLinkerModule(TheModule, DefinedGlobals);
 
     // Save temps: after promotion.
     saveTempBitcode(TheModule, SaveTempsDir, count, ".1.promoted.bc");
   }
 
-  // Internalization
-  doInternalizeModule(TheModule, TM, PreservedGV);
+  // Be friendly and don't nuke totally the module when the client didn't
+  // supply anything to preserve.
+  if (!ExportList.empty() || !GUIDPreservedSymbols.empty()) {
+    // Apply summary-based internalization decisions.
+    thinLTOInternalizeModule(TheModule, DefinedGlobals);
+  }
 
   // Save internalized bitcode
   saveTempBitcode(TheModule, SaveTempsDir, count, ".2.internalized.bc");
 
   if (!SingleModule) {
     crossImportIntoModule(TheModule, Index, ModuleMap, ImportList);
 
     // Save temps: after cross-module import.
SmallVector<char, 128> OutputBuffer;
       WriteBitcodeToFile(&TheModule, OS, true, &IndexBuilder.getIndex());
     }
     return make_unique<ObjectMemoryBuffer>(std::move(OutputBuffer));
   }
 
   return codegenModule(TheModule, TM);
 }
 
+/// Resolve LinkOnce/Weak symbols. Record resolutions in the \p ResolvedODR map
+/// for caching, and in the \p Index for application during the ThinLTO
+/// backends. This is needed for correctness for exported symbols (ensure
+/// at least one copy kept) and a compile-time optimization (to drop duplicate
+/// copies when possible).
+static void resolveWeakForLinkerInIndex(
+    ModuleSummaryIndex &Index,
+    const StringMap<FunctionImporter::ExportSetTy> &ExportLists,
+    const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols,
+    StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>>
+        &ResolvedODR) {
+
+  DenseMap<GlobalValue::GUID, const GlobalValueSummary *> PrevailingCopy;
+  computePrevailingCopies(Index, PrevailingCopy);
+
+  auto isPrevailing = [&](GlobalValue::GUID GUID, const GlobalValueSummary *S) {
+    const auto &Prevailing = PrevailingCopy.find(GUID);
+    // Not in map means that there was only one copy, which must be prevailing.
+    if (Prevailing == PrevailingCopy.end())
+      return true;
+    return Prevailing->second == S;
+  };
+
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    const auto &ExportList = ExportLists.find(ModuleIdentifier);
+    assert(ExportList != ExportLists.end() && "Missing export list for module");
+    return ExportList->second.count(GUID) || GUIDPreservedSymbols.count(GUID);
+  };
+
+  auto recordNewLinkage = [&](StringRef ModuleIdentifier,
+                              GlobalValue::GUID GUID,
+                              GlobalValue::LinkageTypes NewLinkage) {
+    ResolvedODR[ModuleIdentifier][GUID] = NewLinkage;
+  };
+
+  thinLTOResolveWeakForLinkerInIndex(Index, isPrevailing, isExported,
+                                     recordNewLinkage);
+}
+
 // Initialize the TargetMachine builder for a given Triple
 static void initTMBuilder(TargetMachineBuilder &TMBuilder,
                           const Triple &TheTriple) {
   // Set a default CPU for Darwin triples (copied from LTOCodeGenerator).
   // FIXME this looks pretty terrible...
   if (TMBuilder.MCpu.empty() && TheTriple.isOSDarwin()) {
     if (TheTriple.getArch() == llvm::Triple::x86_64)
       TMBuilder.MCpu = "core2";
if (CombinedIndex) {
       CombinedIndex = std::move(Index);
     }
   }
   return CombinedIndex;
 }
 
 /**
  * Perform promotion and renaming of exported internal functions.
+ * Index is updated to reflect linkage changes from weak resolution.
  */
 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);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
                            ExportLists);
-  auto &ExportList = ExportLists[ModuleIdentifier];
 
   // Convert the preserved symbols set from string to GUID
   auto GUIDPreservedSymbols =
   computeGUIDPreservedSymbols(PreservedSymbols, TMBuilder.TheTriple);
 
-  // Resolve the LinkOnceODR, trying to turn them into "available_externally"
-  // where possible.
-  // This is a compile-time optimization.
-  // We use a std::map here to be able to have a defined ordering when
-  // producing a hash for the cache entry.
-  std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
-  ResolveODR(Index, ExportList, GUIDPreservedSymbols, ModuleToDefinedGVSummaries[ModuleIdentifier],
-             ModuleIdentifier, ResolvedODR);
-  fixupODR(TheModule, ResolvedODR);
+  // Resolve LinkOnce/Weak symbols.
+  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
+  resolveWeakForLinkerInIndex(Index, ExportLists, GUIDPreservedSymbols,
+                              ResolvedODR);
+
+  thinLTOResolveWeakForLinkerModule(
+      TheModule, ModuleToDefinedGVSummaries[ModuleIdentifier]);
 
   promoteModule(TheModule, Index);
 }
 
 /**
  * Perform cross-module importing for the module identified by ModuleIdentifier.
  */
 void ThinLTOCodeGenerator::crossModuleImport(Module &TheModule,
void ThinLTOCodeGenerator::emitImports(StringRef ModulePath,
 
   std::error_code EC;
   if ((EC = EmitImportsFiles(ModulePath, OutputName, ImportLists)))
     report_fatal_error(Twine("Failed to open ") + OutputName +
                        " to save imports lists\n");
 }
 
 /**
- * Perform internalization.
+ * 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);
 
   // Generate import/export list
   StringMap<FunctionImporter::ImportMapTy> ImportLists(ModuleCount);
   StringMap<FunctionImporter::ExportSetTy> ExportLists(ModuleCount);
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
                            ExportLists);
   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
-  auto PreservedGV = computePreservedSymbolsForModule(
-      TheModule, GUIDPreservedSymbols, ExportList);
-  doInternalizeModule(TheModule, *TMBuilder.create(), PreservedGV);
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    const auto &ExportList = ExportLists.find(ModuleIdentifier);
+    assert(ExportList != ExportLists.end() && "Missing export list for module");
+    return ExportList->second.count(GUID) || GUIDPreservedSymbols.count(GUID);
+  };
+  thinLTOInternalizeAndPromoteInIndex(Index, isExported);
+  thinLTOInternalizeModule(TheModule,
+                           ModuleToDefinedGVSummaries[ModuleIdentifier]);
 }
 
 /**
  * Perform post-importing ThinLTO optimizations.
  */
 void ThinLTOCodeGenerator::optimize(Module &TheModule) {
   initTMBuilder(TMBuilder, Triple(TheModule.getTargetTriple()));
 
void ThinLTOCodeGenerator::run() {
   ComputeCrossModuleImport(*Index, ModuleToDefinedGVSummaries, ImportLists,
                            ExportLists);
 
   // 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.
+  // FIXME: we should be able to compute the caching hash for the entry based
+  // on the index, and nuke this map.
+  StringMap<std::map<GlobalValue::GUID, GlobalValue::LinkageTypes>> ResolvedODR;
+
+  // Resolve LinkOnce/Weak symbols, this has to be computed early because it
+  // impacts the caching.
+  resolveWeakForLinkerInIndex(*Index, ExportLists, GUIDPreservedSymbols,
+                              ResolvedODR);
+
+  auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
+    const auto &ExportList = ExportLists.find(ModuleIdentifier);
+    assert(ExportList != ExportLists.end() && "Missing export list for module");
+    return ExportList->second.count(GUID) || GUIDPreservedSymbols.count(GUID);
+  };
+
+  // Use global summary-based analysis to identify symbols that can be
+  // internalized (because they aren't exported or preserved as per callback).
+  // Changes are made in the index, consumed in the ThinLTO backends.
+  thinLTOInternalizeAndPromoteInIndex(*Index, isExported);
+
   // Make sure that every module has an entry in the ExportLists to enable
   // threaded access to this map below
   for (auto &DefinedGVSummaries : ModuleToDefinedGVSummaries)
     ExportLists[DefinedGVSummaries.first()];
 
   // Compute the ordering we will process the inputs: the rough heuristic here
   // is to sort them per size so that the largest module get schedule as soon as
   // possible. This is purely a compile-time optimization.
// Parallel optimizer + codegen
     for (auto IndexCount : ModulesOrdering) {
       auto &ModuleBuffer = Modules[IndexCount];
       Pool.async([&](int count) {
         auto ModuleIdentifier = ModuleBuffer.getBufferIdentifier();
         auto &ExportList = ExportLists[ModuleIdentifier];
 
         auto &DefinedFunctions = ModuleToDefinedGVSummaries[ModuleIdentifier];
 
-        // Resolve ODR, this has to be done early because it impacts the caching
-        // We use a std::map here to be able to have a defined ordering when
-        // producing a hash for the cache entry.
-        std::map<GlobalValue::GUID, GlobalValue::LinkageTypes> ResolvedODR;
-        ResolveODR(*Index, ExportList, GUIDPreservedSymbols, DefinedFunctions, ModuleIdentifier,
-                   ResolvedODR);
-
         // The module may be cached, this helps handling it.
         ModuleCacheEntry CacheEntry(CacheOptions.Path, *Index, ModuleIdentifier,
                                     ImportLists[ModuleIdentifier], ExportList,
-                                    ResolvedODR, DefinedFunctions,
-                                    GUIDPreservedSymbols);
+                                    ResolvedODR[ModuleIdentifier],
+                                    DefinedFunctions, GUIDPreservedSymbols);
 
         {
           auto ErrOrBuffer = CacheEntry.tryLoadingBuffer();
           DEBUG(dbgs() << "Cache " << (ErrOrBuffer ? "hit" : "miss") << " '"
                        << CacheEntry.getEntryPath() << "' for buffer " << count
                        << " " << ModuleIdentifier << "\n");
 
           if (ErrOrBuffer) {
for (auto IndexCount : ModulesOrdering) {
 
         // Save temps: original file.
         saveTempBitcode(*TheModule, SaveTempsDir, count, ".0.original.bc");
 
         auto &ImportList = ImportLists[ModuleIdentifier];
         // Run the main process now, and generates a binary
         auto OutputBuffer = ProcessThinLTOModule(
             *TheModule, *Index, ModuleMap, *TMBuilder.create(), ImportList,
-            ExportList, GUIDPreservedSymbols, ResolvedODR, CacheOptions,
+            ExportList, GUIDPreservedSymbols,
+            ModuleToDefinedGVSummaries[ModuleIdentifier], CacheOptions,
             DisableCodeGen, SaveTempsDir, count);
 
         OutputBuffer = CacheEntry.write(std::move(OutputBuffer));
         ProducedBinaries[count] = std::move(OutputBuffer);
       }, IndexCount);
     }
   }