Change GlobalValueSummaryMapTy from std::map to DenseMap.

[danielcdh]

Summary

GlobalValueSummaryMapTy is heavily accessed datasture. And it is large thus nlogn algorithm becomes quite expensive. Changing this to DenseMap speeds up thin-link by ~36% when building large applications.

[dblaikie]

include/llvm/Analysis/ModuleSummaryAnalysis.h

32

I take it this change was necessary because buildModuleSummaryIndex indirectly emits references to the elements of the map? (& DenseMap doesn't preserve reference/pointer validity over move? That surprises me slightly, but I could believe it)

Is that correct? (otherwise - what's the motivation for this change?)

This should be unique_ptr<ModuleSummaryIndex> not raw-but-owning pointer ModuleSummaryIndex*.

include/llvm/IR/ModuleSummaryIndex.h

88–114

I take it this change is necessary because ValueInfo elements are stored over mutations to the ModuleSummaryIndex & thus would invalidate references/pointers/iterators to elements & so the map lookup must be deferred?

Is that correct? Is there a particular/canonical/unavoidable example of this usage?

lib/LTO/LTO.cpp

1058

This looks like an independent change? Should it be submitted separately with the value/memory usage/runtime performance improvements appropriately credited (so it's clear how much of the benefit is from this change as opposed to the ModuleSummaryIndex change?)

include/llvm/Analysis/ModuleSummaryAnalysis.h

 class ProfileSummaryInfo;
 
 /// Direct function to compute a \c ModuleSummaryIndex from a given module.
 ///
 /// If operating within a pass manager which has defined ways to compute the \c
 /// BlockFrequencyInfo for a given function, that can be provided via
 /// a std::function callback. Otherwise, this routine will manually construct
 /// that information.
-ModuleSummaryIndex buildModuleSummaryIndex(
+ModuleSummaryIndex *buildModuleSummaryIndex(
     const Module &M,
     std::function<BlockFrequencyInfo *(const Function &F)> GetBFICallback,
     ProfileSummaryInfo *PSI);
 
 /// Analysis pass to provide the ModuleSummaryIndex object.
 class ModuleSummaryIndexAnalysis
     : public AnalysisInfoMixin<ModuleSummaryIndexAnalysis> {
   friend AnalysisInfoMixin<ModuleSummaryIndexAnalysis>;
   static AnalysisKey Key;
 
 public:
-  typedef ModuleSummaryIndex Result;
+  typedef std::unique_ptr<ModuleSummaryIndex> Result;
 
   Result run(Module &M, ModuleAnalysisManager &AM);
 };
 
 /// Legacy wrapper pass to provide the ModuleSummaryIndex object.
 class ModuleSummaryIndexWrapperPass : public ModulePass {
-  Optional<ModuleSummaryIndex> Index;
+  std::unique_ptr<ModuleSummaryIndex> Index;
 
 public:
   static char ID;
 
   ModuleSummaryIndexWrapperPass();
 
   /// Get the index built by pass
   ModuleSummaryIndex &getIndex() { return *Index; }

include/llvm/IR/ModuleSummaryIndex.h

struct GlobalValueSummaryInfo {
   const GlobalValue *GV = nullptr;
 
   /// List of global value summary structures for a particular value held
   /// in the GlobalValueMap. Requires a vector in the case of multiple
   /// COMDAT values of the same name.
   GlobalValueSummaryList SummaryList;
 };
 
-/// Map from global value GUID to corresponding summary structures. Use a
-/// std::map rather than a DenseMap so that pointers to the map's value_type
-/// (which are used by ValueInfo) are not invalidated by insertion. Also it will
-/// likely incur less overhead, as the value type is not very small and the size
-/// of the map is unknown, resulting in inefficiencies due to repeated
-/// insertions and resizing.
+/// Map from global value GUID to corresponding summary structures.
 using GlobalValueSummaryMapTy =
-    std::map<GlobalValue::GUID, GlobalValueSummaryInfo>;
+    DenseMap<GlobalValue::GUID, GlobalValueSummaryInfo>;
 
 /// Struct that holds a reference to a particular GUID in a global value
 /// summary.
 struct ValueInfo {
-  const GlobalValueSummaryMapTy::value_type *Ref = nullptr;
+  GlobalValue::GUID GUID = 0;
+  const GlobalValueSummaryMapTy *Map = nullptr;
 
   ValueInfo() = default;
-  ValueInfo(const GlobalValueSummaryMapTy::value_type *Ref) : Ref(Ref) {}
+  ValueInfo(GlobalValue::GUID GUID, const GlobalValueSummaryMapTy *Map)
+      : GUID(GUID), Map(Map) {}
 
-  operator bool() const { return Ref; }
+  operator bool() const { return Map; }
 
-  GlobalValue::GUID getGUID() const { return Ref->first; }
-  const GlobalValue *getValue() const { return Ref->second.GV; }
+  GlobalValue::GUID getGUID() const { return GUID; }
+  const GlobalValue *getValue() const {
+    if (Map == nullptr)
+      return nullptr;
+    auto I = Map->find(GUID);
+    return I == Map->end() ? nullptr : I->second.GV;
+  }
 
   ArrayRef<std::unique_ptr<GlobalValueSummary>> getSummaryList() const {
-    return Ref->second.SummaryList;
+    return Map->find(GUID)->second.SummaryList;
+  }
+
+  void addSummary(std::unique_ptr<GlobalValueSummary> Summary) {
+    const_cast<GlobalValueSummaryList &>(Map->find(GUID)->second.SummaryList)
+        .push_back(std::move(Summary));
   }
 };
 
 template <> struct DenseMapInfo<ValueInfo> {
   static inline ValueInfo getEmptyKey() {
-    return ValueInfo((GlobalValueSummaryMapTy::value_type *)-1);
+    return ValueInfo(-1, (const GlobalValueSummaryMapTy *)-1);
   }
 
   static inline ValueInfo getTombstoneKey() {
-    return ValueInfo((GlobalValueSummaryMapTy::value_type *)-2);
+    return ValueInfo(-2, (const GlobalValueSummaryMapTy *)-2);
   }
 
-  static bool isEqual(ValueInfo L, ValueInfo R) { return L.Ref == R.Ref; }
-  static unsigned getHashValue(ValueInfo I) { return (uintptr_t)I.Ref; }
+  static bool isEqual(ValueInfo L, ValueInfo R) {
+    return L.GUID == R.GUID && L.Map == R.Map;
+  }
+  static unsigned getHashValue(ValueInfo I) { return I.GUID; }
 };
 
 /// \brief Function and variable summary information to aid decisions and
 /// implementation of importing.
 class GlobalValueSummary {
 public:
   /// \brief Sububclass discriminator (for dyn_cast<> et al.)
   enum SummaryKind : unsigned { AliasKind, FunctionKind, GlobalVarKind };
private:
 
   /// Mapping from type identifiers to summary information for that type
   /// identifier.
   // FIXME: Add bitcode read/write support for this field.
   std::map<std::string, TypeIdSummary> TypeIdMap;
 
   /// Mapping from original ID to GUID. If original ID can map to multiple
   /// GUIDs, it will be mapped to 0.
-  std::map<GlobalValue::GUID, GlobalValue::GUID> OidGuidMap;
+  DenseMap<GlobalValue::GUID, GlobalValue::GUID> OidGuidMap;
 
   /// Indicates that summary-based GlobalValue GC has run, and values with
   /// GVFlags::Live==false are really dead. Otherwise, all values must be
   /// considered live.
   bool WithGlobalValueDeadStripping = false;
 
   std::set<std::string> CfiFunctionDefs;
   std::set<std::string> CfiFunctionDecls;
 
   // YAML I/O support.
   friend yaml::MappingTraits<ModuleSummaryIndex>;
 
   GlobalValueSummaryMapTy::value_type *
   getOrInsertValuePtr(GlobalValue::GUID GUID) {
-    return &*GlobalValueMap.emplace(GUID, GlobalValueSummaryInfo{}).first;
+    return &*GlobalValueMap
+                 .insert(std::make_pair(GUID, GlobalValueSummaryInfo{}))
+                 .first;
   }
 
 public:
   gvsummary_iterator begin() { return GlobalValueMap.begin(); }
   const_gvsummary_iterator begin() const { return GlobalValueMap.begin(); }
   gvsummary_iterator end() { return GlobalValueMap.end(); }
   const_gvsummary_iterator end() const { return GlobalValueMap.end(); }
   size_t size() const { return GlobalValueMap.size(); }
 
   bool withGlobalValueDeadStripping() const {
     return WithGlobalValueDeadStripping;
   }
   void setWithGlobalValueDeadStripping() {
     WithGlobalValueDeadStripping = true;
   }
 
   bool isGlobalValueLive(const GlobalValueSummary *GVS) const {
     return !WithGlobalValueDeadStripping || GVS->isLive();
   }
   bool isGUIDLive(GlobalValue::GUID GUID) const;
 
   /// Return a ValueInfo for GUID if it exists, otherwise return ValueInfo().
   ValueInfo getValueInfo(GlobalValue::GUID GUID) const {
     auto I = GlobalValueMap.find(GUID);
-    return ValueInfo(I == GlobalValueMap.end() ? nullptr : &*I);
+    return ValueInfo(GUID,
+                     I == GlobalValueMap.end() ? nullptr : &GlobalValueMap);
   }
 
   /// Return a ValueInfo for \p GUID.
   ValueInfo getOrInsertValueInfo(GlobalValue::GUID GUID) {
-    return ValueInfo(getOrInsertValuePtr(GUID));
+    getOrInsertValuePtr(GUID);
+    return ValueInfo(GUID, &GlobalValueMap);
   }
 
   /// Return a ValueInfo for \p GV and mark it as belonging to GV.
   ValueInfo getOrInsertValueInfo(const GlobalValue *GV) {
     auto VP = getOrInsertValuePtr(GV->getGUID());
     VP->second.GV = GV;
-    return ValueInfo(VP);
+    return ValueInfo(GV->getGUID(), &GlobalValueMap);
   }
 
   /// Return the GUID for \p OriginalId in the OidGuidMap.
   GlobalValue::GUID getGUIDFromOriginalID(GlobalValue::GUID OriginalID) const {
     const auto I = OidGuidMap.find(OriginalID);
     return I == OidGuidMap.end() ? 0 : I->second;
   }
 
void addGlobalValueSummary(StringRef ValueName,
     addGlobalValueSummary(getOrInsertValueInfo(GlobalValue::getGUID(ValueName)),
                           std::move(Summary));
   }
 
   /// Add a global value summary for the given ValueInfo.
   void addGlobalValueSummary(ValueInfo VI,
                              std::unique_ptr<GlobalValueSummary> Summary) {
     addOriginalName(VI.getGUID(), Summary->getOriginalName());
-    // Here we have a notionally const VI, but the value it points to is owned
-    // by the non-const *this.
-    const_cast<GlobalValueSummaryMapTy::value_type *>(VI.Ref)
-        ->second.SummaryList.push_back(std::move(Summary));
+    VI.addSummary(std::move(Summary));
   }
 
   /// Add an original name for the value of the given GUID.
   void addOriginalName(GlobalValue::GUID ValueGUID,
                        GlobalValue::GUID OrigGUID) {
     if (OrigGUID == 0 || ValueGUID == OrigGUID)
       return;
     if (OidGuidMap.count(OrigGUID) && OidGuidMap[OrigGUID] != ValueGUID)

lib/Analysis/ModuleSummaryAnalysis.cpp

 
 // Set LiveRoot flag on entries matching the given value name.
 static void setLiveRoot(ModuleSummaryIndex &Index, StringRef Name) {
   if (ValueInfo VI = Index.getValueInfo(GlobalValue::getGUID(Name)))
     for (auto &Summary : VI.getSummaryList())
       Summary->setLive(true);
 }
 
-ModuleSummaryIndex llvm::buildModuleSummaryIndex(
+ModuleSummaryIndex *llvm::buildModuleSummaryIndex(
     const Module &M,
     std::function<BlockFrequencyInfo *(const Function &F)> GetBFICallback,
     ProfileSummaryInfo *PSI) {
   assert(PSI);
-  ModuleSummaryIndex Index;
+  ModuleSummaryIndex *Index = new ModuleSummaryIndex();
 
   // Identify the local values in the llvm.used and llvm.compiler.used sets,
   // which should not be exported as they would then require renaming and
   // promotion, but we may have opaque uses e.g. in inline asm. We collect them
   // here because we use this information to mark functions containing inline
   // assembly calls as not importable.
   SmallPtrSet<GlobalValue *, 8> LocalsUsed;
   SmallPtrSet<GlobalValue *, 8> Used;
if (GetBFICallback)
       BFI = GetBFICallback(F);
     else if (F.getEntryCount().hasValue()) {
       LoopInfo LI{DominatorTree(const_cast<Function &>(F))};
       BranchProbabilityInfo BPI{F, LI};
       BFIPtr = llvm::make_unique<BlockFrequencyInfo>(F, BPI, LI);
       BFI = BFIPtr.get();
     }
 
-    computeFunctionSummary(Index, M, F, BFI, PSI, !LocalsUsed.empty(),
+    computeFunctionSummary(*Index, M, F, BFI, PSI, !LocalsUsed.empty(),
                            CantBePromoted);
   }
 
   // Compute summaries for all variables defined in module, and save in the
   // index.
   for (const GlobalVariable &G : M.globals()) {
     if (G.isDeclaration())
       continue;
-    computeVariableSummary(Index, G, CantBePromoted);
+    computeVariableSummary(*Index, G, CantBePromoted);
   }
 
   // Compute summaries for all aliases defined in module, and save in the
   // index.
   for (const GlobalAlias &A : M.aliases())
-    computeAliasSummary(Index, A, CantBePromoted);
+    computeAliasSummary(*Index, A, CantBePromoted);
 
   for (auto *V : LocalsUsed) {
-    auto *Summary = Index.getGlobalValueSummary(*V);
+    auto *Summary = Index->getGlobalValueSummary(*V);
     assert(Summary && "Missing summary for global value");
     Summary->setNotEligibleToImport();
   }
 
   // The linker doesn't know about these LLVM produced values, so we need
   // to flag them as live in the index to ensure index-based dead value
   // analysis treats them as live roots of the analysis.
-  setLiveRoot(Index, "llvm.used");
-  setLiveRoot(Index, "llvm.compiler.used");
-  setLiveRoot(Index, "llvm.global_ctors");
-  setLiveRoot(Index, "llvm.global_dtors");
-  setLiveRoot(Index, "llvm.global.annotations");
+  setLiveRoot(*Index, "llvm.used");
+  setLiveRoot(*Index, "llvm.compiler.used");
+  setLiveRoot(*Index, "llvm.global_ctors");
+  setLiveRoot(*Index, "llvm.global_dtors");
+  setLiveRoot(*Index, "llvm.global.annotations");
 
   if (!M.getModuleInlineAsm().empty()) {
     // Collect the local values defined by module level asm, and set up
     // summaries for these symbols so that they can be marked as NoRename,
     // to prevent export of any use of them in regular IR that would require
     // renaming within the module level asm. Note we don't need to create a
     // summary for weak or global defs, as they don't need to be flagged as
     // NoRename, and defs in module level asm can't be imported anyway.
     // Also, any values used but not defined within module level asm should
     // be listed on the llvm.used or llvm.compiler.used global and marked as
     // referenced from there.
     ModuleSymbolTable::CollectAsmSymbols(
-        M, [&M, &Index, &CantBePromoted](StringRef Name,
+        M, [&M, Index, &CantBePromoted](StringRef Name,
                                          object::BasicSymbolRef::Flags Flags) {
           // Symbols not marked as Weak or Global are local definitions.
           if (Flags & (object::BasicSymbolRef::SF_Weak |
                        object::BasicSymbolRef::SF_Global))
             return;
           GlobalValue *GV = M.getNamedValue(Name);
           if (!GV)
             return;
           assert(GV->isDeclaration() && "Def in module asm already has definition");
           GlobalValueSummary::GVFlags GVFlags(GlobalValue::InternalLinkage,
                                               /* NotEligibleToImport = */ true,
                                               /* Live = */ true);
           CantBePromoted.insert(GlobalValue::getGUID(Name));
           // Create the appropriate summary type.
           if (isa<Function>(GV)) {
             std::unique_ptr<FunctionSummary> Summary =
                 llvm::make_unique<FunctionSummary>(
                     GVFlags, 0, ArrayRef<ValueInfo>{},
                     ArrayRef<FunctionSummary::EdgeTy>{},
                     ArrayRef<GlobalValue::GUID>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::VFuncId>{},
                     ArrayRef<FunctionSummary::ConstVCall>{},
                     ArrayRef<FunctionSummary::ConstVCall>{});
-            Index.addGlobalValueSummary(Name, std::move(Summary));
+            Index->addGlobalValueSummary(Name, std::move(Summary));
           } else {
             std::unique_ptr<GlobalVarSummary> Summary =
                 llvm::make_unique<GlobalVarSummary>(GVFlags,
                                                     ArrayRef<ValueInfo>{});
-            Index.addGlobalValueSummary(Name, std::move(Summary));
+            Index->addGlobalValueSummary(Name, std::move(Summary));
           }
         });
   }
 
   bool IsThinLTO = true;
   if (auto *MD =
           mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("ThinLTO")))
     IsThinLTO = MD->getZExtValue();
 
-  for (auto &GlobalList : Index) {
+  for (auto &GlobalList : *Index) {
     // Ignore entries for references that are undefined in the current module.
     if (GlobalList.second.SummaryList.empty())
       continue;
 
     assert(GlobalList.second.SummaryList.size() == 1 &&
            "Expected module's index to have one summary per GUID");
     auto &Summary = GlobalList.second.SummaryList[0];
     if (!IsThinLTO) {
for (auto &GlobalList : *Index) {
     }
   }
 
   return Index;
 }
 
 AnalysisKey ModuleSummaryIndexAnalysis::Key;
 
-ModuleSummaryIndex
+std::unique_ptr<ModuleSummaryIndex>
 ModuleSummaryIndexAnalysis::run(Module &M, ModuleAnalysisManager &AM) {
   ProfileSummaryInfo &PSI = AM.getResult<ProfileSummaryAnalysis>(M);
   auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
-  return buildModuleSummaryIndex(
+  return std::unique_ptr<ModuleSummaryIndex>(buildModuleSummaryIndex(
       M,
       [&FAM](const Function &F) {
         return &FAM.getResult<BlockFrequencyAnalysis>(
             *const_cast<Function *>(&F));
       },
-      &PSI);
+      &PSI));
 }
 
 char ModuleSummaryIndexWrapperPass::ID = 0;
 INITIALIZE_PASS_BEGIN(ModuleSummaryIndexWrapperPass, "module-summary-analysis",
                       "Module Summary Analysis", false, true)
 INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
 INITIALIZE_PASS_END(ModuleSummaryIndexWrapperPass, "module-summary-analysis",
                     "Module Summary Analysis", false, true)
 
 ModulePass *llvm::createModuleSummaryIndexWrapperPass() {
   return new ModuleSummaryIndexWrapperPass();
 }
 
 ModuleSummaryIndexWrapperPass::ModuleSummaryIndexWrapperPass()
     : ModulePass(ID) {
   initializeModuleSummaryIndexWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
 bool ModuleSummaryIndexWrapperPass::runOnModule(Module &M) {
   auto &PSI = *getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
-  Index = buildModuleSummaryIndex(
+  Index.reset(buildModuleSummaryIndex(
       M,
       [this](const Function &F) {
         return &(this->getAnalysis<BlockFrequencyInfoWrapperPass>(
                          *const_cast<Function *>(&F))
                      .getBFI());
       },
-      &PSI);
+      &PSI));
   return false;
 }
 
 bool ModuleSummaryIndexWrapperPass::doFinalization(Module &M) {
-  Index.reset();
   return false;
 }
 
 void ModuleSummaryIndexWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesAll();
   AU.addRequired<BlockFrequencyInfoWrapperPass>();
   AU.addRequired<ProfileSummaryInfoWrapperPass>();
 }

lib/Bitcode/Writer/BitcodeWriterPass.cpp

 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/Pass.h"
 using namespace llvm;
 
 PreservedAnalyses BitcodeWriterPass::run(Module &M, ModuleAnalysisManager &AM) {
   const ModuleSummaryIndex *Index =
-      EmitSummaryIndex ? &(AM.getResult<ModuleSummaryIndexAnalysis>(M))
+      EmitSummaryIndex ? AM.getResult<ModuleSummaryIndexAnalysis>(M).get()
                        : nullptr;
   WriteBitcodeToFile(&M, OS, ShouldPreserveUseListOrder, Index, EmitModuleHash);
   return PreservedAnalyses::all();
 }
 
 namespace {
   class WriteBitcodePass : public ModulePass {
     raw_ostream &OS; // raw_ostream to print on

lib/LTO/LTO.cpp

Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
   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);
 
-    std::set<GlobalValue::GUID> ExportedGUIDs;
+    DenseSet<GlobalValue::GUID> ExportedGUIDs;
     for (auto &Res : GlobalResolutions) {
       // First check if the symbol was flagged as having external references.
       if (Res.second.Partition != GlobalResolution::External)
         continue;
       // IRName will be defined if we have seen the prevailing copy of
       // this value. If not, no need to mark as exported from a ThinLTO
       // partition (and we can't get the GUID).
       if (Res.second.IRName.empty())

lib/LTO/ThinLTOCodeGenerator.cpp

ProcessThinLTOModule(Module &TheModule, ModuleSummaryIndex &Index,
   saveTempBitcode(TheModule, SaveTempsDir, count, ".4.opt.bc");
 
   if (DisableCodeGen) {
     // Configured to stop before CodeGen, serialize the bitcode and return.
     SmallVector<char, 128> OutputBuffer;
     {
       raw_svector_ostream OS(OutputBuffer);
       ProfileSummaryInfo PSI(TheModule);
-      auto Index = buildModuleSummaryIndex(TheModule, nullptr, &PSI);
-      WriteBitcodeToFile(&TheModule, OS, true, &Index);
+      std::unique_ptr<ModuleSummaryIndex> Index(
+          buildModuleSummaryIndex(TheModule, nullptr, &PSI));
+      WriteBitcodeToFile(&TheModule, OS, true, Index.get());
     }
     return make_unique<ObjectMemoryBuffer>(std::move(OutputBuffer));
   }
 
   return codegenModule(TheModule, TM);
 }
 
 /// Resolve LinkOnce/Weak symbols. Record resolutions in the \p ResolvedODR map

lib/Transforms/IPO/FunctionImport.cpp

for (auto &S : VI.getSummaryList())
       S->setLive(true);
   }
 
   // Add values flagged in the index as live roots to the worklist.
   for (const auto &Entry : Index)
     for (auto &S : Entry.second.SummaryList)
       if (S->isLive()) {
         DEBUG(dbgs() << "Live root: " << Entry.first << "\n");
-        Worklist.push_back(ValueInfo(&Entry));
+        Worklist.push_back(Index.getValueInfo(Entry.first));
         ++LiveSymbols;
         break;
       }
 
   // Make value live and add it to the worklist if it was not live before.
   // FIXME: we should only make the prevailing copy live here
   auto visit = [&](ValueInfo VI) {
     for (auto &S : VI.getSummaryList())

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

if(!CfiFunctionMDs.empty()) {
     for (auto MD : CfiFunctionMDs)
       NMD->addOperand(MD);
   }
 
   simplifyExternals(*MergedM);
 
   // FIXME: Try to re-use BSI and PFI from the original module here.
   ProfileSummaryInfo PSI(M);
-  ModuleSummaryIndex Index = buildModuleSummaryIndex(M, nullptr, &PSI);
+  std::unique_ptr<ModuleSummaryIndex> Index(
+      buildModuleSummaryIndex(M, nullptr, &PSI));
 
   // Mark the merged module as requiring full LTO. We still want an index for
   // it though, so that it can participate in summary-based dead stripping.
   MergedM->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-  ModuleSummaryIndex MergedMIndex =
-      buildModuleSummaryIndex(*MergedM, nullptr, &PSI);
+  std::unique_ptr<ModuleSummaryIndex> MergedMIndex(
+      buildModuleSummaryIndex(*MergedM, nullptr, &PSI));
 
   SmallVector<char, 0> Buffer;
 
   BitcodeWriter W(Buffer);
   // Save the module hash produced for the full bitcode, which will
   // be used in the backends, and use that in the minimized bitcode
   // produced for the full link.
   ModuleHash ModHash = {{0}};
-  W.writeModule(&M, /*ShouldPreserveUseListOrder=*/false, &Index,
+  W.writeModule(&M, /*ShouldPreserveUseListOrder=*/false, Index.get(),
                 /*GenerateHash=*/true, &ModHash);
   W.writeModule(MergedM.get(), /*ShouldPreserveUseListOrder=*/false,
-                &MergedMIndex);
+                MergedMIndex.get());
   W.writeSymtab();
   W.writeStrtab();
   OS << Buffer;
 
   // If a minimized bitcode module was requested for the thin link,
   // strip the debug info (the merged module was already stripped above)
   // and write it to the given OS.
   if (ThinLinkOS) {
     Buffer.clear();
     BitcodeWriter W2(Buffer);
     StripDebugInfo(M);
-    W2.writeModule(&M, /*ShouldPreserveUseListOrder=*/false, &Index,
+    W2.writeModule(&M, /*ShouldPreserveUseListOrder=*/false, Index.get(),
                    /*GenerateHash=*/false, &ModHash);
     W2.writeModule(MergedM.get(), /*ShouldPreserveUseListOrder=*/false,
-                   &MergedMIndex);
+                   MergedMIndex.get());
     W2.writeSymtab();
     W2.writeStrtab();
     *ThinLinkOS << Buffer;
   }
 }
 
 // Returns whether this module needs to be split because it uses type metadata.
 bool requiresSplit(Module &M) {

 PreservedAnalyses
 llvm::ThinLTOBitcodeWriterPass::run(Module &M, ModuleAnalysisManager &AM) {
   FunctionAnalysisManager &FAM =
       AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
   writeThinLTOBitcode(OS, ThinLinkOS,
                       [&FAM](Function &F) -> AAResults & {
                         return FAM.getResult<AAManager>(F);
                       },
-                      M, &AM.getResult<ModuleSummaryIndexAnalysis>(M));
+                      M, AM.getResult<ModuleSummaryIndexAnalysis>(M).get());
   return PreservedAnalyses::all();
 }

test/Bitcode/thinlto-alias.ll

 ; CHECK-NEXT: blob data = 'mainanalias{{.*}}'
 
 ; COMBINED:       <GLOBALVAL_SUMMARY_BLOCK
 ; COMBINED-NEXT:    <VERSION
 ; See if the call to analias is registered, using the expected value id.
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[ALIASID:[0-9]+]] op1=-5751648690987223394/>
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID op0=[[ALIASEEID:[0-9]+]] op1=-1039159065113703048/>
-; COMBINED-NEXT:    <COMBINED {{.*}} op5=[[ALIASID]]/>
 ; COMBINED-NEXT:    <COMBINED {{.*}}
+; COMBINED-NEXT:    <COMBINED {{.*}} op5=[[ALIASID]]/>
 ; COMBINED-NEXT:    <COMBINED_ALIAS  {{.*}} op3=[[ALIASEEID]]
 ; COMBINED-NEXT:  </GLOBALVAL_SUMMARY_BLOCK
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: nounwind uwtable
 define i32 @main() {
 entry:
     call void (...) @analias()
     ret i32 0
 }
 
 declare void @analias(...)

test/Bitcode/thinlto-function-summary-callgraph-profile-summary.ll

 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
+; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op5=[[HOT1:.*]] op6=3 op7=[[COLD:.*]] op8=1 op9=[[HOT2:.*]] op10=3 op11=[[NONE1:.*]] op12=2 op13=[[HOT3:.*]] op14=3 op15=[[NONE2:.*]] op16=2 op17=[[NONE3:.*]] op18=2/>
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
-; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op5=[[HOT1:.*]] op6=3 op7=[[COLD:.*]] op8=1 op9=[[HOT2:.*]] op10=3 op11=[[NONE1:.*]] op12=2 op13=[[HOT3:.*]] op14=3 op15=[[NONE2:.*]] op16=2 op17=[[NONE3:.*]] op18=2/>
 ; COMBINED_NEXT:    <COMBINED abbrevid=
 ; COMBINED_NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

test/Bitcode/thinlto-function-summary-callgraph-sample-profile-summary.ll

 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <VALUE_GUID
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
+; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op5=[[HOT1:.*]] op6=3 op7=[[COLD:.*]] op8=1 op9=[[HOT2:.*]] op10=3 op11=[[NONE1:.*]] op12=2 op13=[[HOT3:.*]] op14=3 op15=[[NONE2:.*]] op16=2 op17=[[NONE3:.*]] op18=2/>
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
 ; COMBINED-NEXT:    <COMBINED abbrevid=
-; COMBINED-NEXT:    <COMBINED_PROFILE {{.*}} op5=[[HOT1:.*]] op6=3 op7=[[COLD:.*]] op8=1 op9=[[HOT2:.*]] op10=3 op11=[[NONE1:.*]] op12=2 op13=[[HOT3:.*]] op14=3 op15=[[NONE2:.*]] op16=2 op17=[[NONE3:.*]] op18=2/>
 ; COMBINED_NEXT:    <COMBINED abbrevid=
 ; COMBINED_NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
 
 
 ; ModuleID = 'thinlto-function-summary-callgraph.ll'
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 

test/Bitcode/thinlto-type-tests.ll

 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
 ; RUN: llvm-lto -thinlto -o %t2 %t.o
 ; RUN: llvm-bcanalyzer -dump %t2.thinlto.bc | FileCheck --check-prefix=COMBINED %s
 
 ; COMBINED: <TYPE_TESTS op0=-2012135647395072713/>
-; COMBINED: <TYPE_TESTS op0=6699318081062747564 op1=-2012135647395072713/>
 ; COMBINED: <TYPE_TESTS op0=6699318081062747564/>
+; COMBINED: <TYPE_TESTS op0=6699318081062747564 op1=-2012135647395072713/>
 
 ; CHECK: <TYPE_TESTS op0=6699318081062747564/>
 define i1 @f() {
   %p = call i1 @llvm.type.test(i8* null, metadata !"foo")
   ret i1 %p
 }
 
 ; CHECK: <TYPE_TESTS op0=6699318081062747564 op1=-2012135647395072713/>

test/Bitcode/thinlto-type-vcalls.ll

 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
 ; RUN: llvm-lto -thinlto -o %t2 %t.o
 ; RUN: llvm-bcanalyzer -dump %t2.thinlto.bc | FileCheck --check-prefix=COMBINED %s
 
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-; COMBINED:      <TYPE_TEST_ASSUME_VCALLS op0=6699318081062747564 op1=16/>
-; COMBINED-NEXT: <COMBINED
-; COMBINED-NEXT: <TYPE_CHECKED_LOAD_VCALLS op0=6699318081062747564 op1=16/>
+; COMBINED: <TYPE_CHECKED_LOAD_CONST_VCALL op0=6699318081062747564 op1=16 op2=42/>
 ; COMBINED-NEXT: <COMBINED
 ; COMBINED-NEXT: <TYPE_TEST_ASSUME_VCALLS op0=6699318081062747564 op1=24 op2=-2012135647395072713 op3=32/>
 ; COMBINED-NEXT: <COMBINED
+; COMBINED-NEXT: <TYPE_TEST_ASSUME_VCALLS op0=6699318081062747564 op1=16/>
+; COMBINED-NEXT: <COMBINED
+; COMBINED-NEXT: <TYPE_CHECKED_LOAD_VCALLS op0=6699318081062747564 op1=16/>
+; COMBINED-NEXT: <COMBINED
 ; COMBINED-NEXT: <TYPE_TEST_ASSUME_CONST_VCALL op0=6699318081062747564 op1=16 op2=42/>
 ; COMBINED-NEXT: <TYPE_TEST_ASSUME_CONST_VCALL op0=6699318081062747564 op1=24 op2=43/>
 ; COMBINED-NEXT: <COMBINED
-; COMBINED-NEXT: <TYPE_CHECKED_LOAD_CONST_VCALL op0=6699318081062747564 op1=16 op2=42/>
-; COMBINED-NEXT: <COMBINED
 ; COMBINED-NEXT: <TYPE_TESTS op0=7546896869197086323/>
 ; COMBINED-NEXT: <COMBINED
 
 ; CHECK: <TYPE_TEST_ASSUME_VCALLS op0=6699318081062747564 op1=16/>
 define void @f1([3 x i8*]* %vtable) {
   %vtablei8 = bitcast [3 x i8*]* %vtable to i8*
   %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"foo")
   call void @llvm.assume(i1 %p)