[LTO] Remove module id from summary index

[tejohnson]

Summary

The module paths string table mapped to both an id sequentially assigned
during LTO linking, and the module hash. The former is leftover from
before the module hash was added for caching and subsequently replaced
use of the module id when renaming promoted symbols (to avoid affects
due to link order changes). The sequentially assigned module id was not
removed, however, as it was still a convenience when serializing to/from
bitcode and assembly.

This patch removes the module id from this table, since it isn't
strictly needed and can lead to confusion on when it is appropriate to
use (e.g. see fix in D156525). It also takes a (likely not significant)
amount of overhead. Where an integer module id is needed (e.g. bitcode
writing), one is assigned on the fly.

There are a couple of test changes since the paths are now sorted
alphanumerically when assigning ids on the fly during assembly writing,
in order to ensure deterministic behavior.

[MaskRay]

Sorry for the delay. Looks great!

llvm/lib/IR/ModuleSummaryIndex.cpp

566

There is a pitfall. ModuleIdMap[ModPath] increases the size while ModuleIdMap.size(); changes the size, so the assignment behavior is dependent on the evaluation order. Suggest try_emplace

[tejohnson]

address comment

[MaskRay]

llvm/lib/IR/ModuleSummaryIndex.cpp

563

llvm::sort(ModulePaths)

[tejohnson]

Address comment

lld/test/COFF/thinlto-index-only.ll

 ; Thin archive tests. Check that the module paths point to the original files.
 ; RUN: rm -rf %t
 ; RUN: mkdir %t
 ; RUN: opt -thinlto-bc -o %t/foo.obj < %s
 ; RUN: opt -thinlto-bc -o %t/bar.obj < %p/Inputs/thinlto.ll
 ; RUN: llvm-ar rcsT %t5.lib %t/bar.obj %t3.obj
 ; RUN: lld-link -thinlto-index-only -entry:main %t/foo.obj %t5.lib
 ; RUN: llvm-dis -o - %t/foo.obj.thinlto.bc | FileCheck %s --check-prefix=THINARCHIVE
-; THINARCHIVE: ^0 = module: (path: "{{.*}}foo.obj",
-; THINARCHIVE: ^1 = module: (path: "{{.*}}bar.obj",
+; THINARCHIVE: ^0 = module: (path: "{{.*}}bar.obj",
+; THINARCHIVE: ^1 = module: (path: "{{.*}}foo.obj",
 
 target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc19.0.24215"
 
 declare void @g(...)
 
 define void @main() {
   call void (...) @g()
   ret void
 }

llvm/include/llvm/Bitcode/BitcodeReader.h

public:
 
     /// Parse the specified bitcode buffer, returning the module summary index.
     Expected<std::unique_ptr<ModuleSummaryIndex>> getSummary();
 
     /// Parse the specified bitcode buffer and merge its module summary index
     /// into CombinedIndex.
     Error
     readSummary(ModuleSummaryIndex &CombinedIndex, StringRef ModulePath,
-                uint64_t ModuleId,
                 std::function<bool(GlobalValue::GUID)> IsPrevailing = nullptr);
   };
 
   struct BitcodeFileContents {
     std::vector<BitcodeModule> Mods;
     StringRef Symtab, StrtabForSymtab;
   };
 
struct ParserCallbacks {
   Expected<BitcodeLTOInfo> getBitcodeLTOInfo(MemoryBufferRef Buffer);
 
   /// Parse the specified bitcode buffer, returning the module summary index.
   Expected<std::unique_ptr<ModuleSummaryIndex>>
   getModuleSummaryIndex(MemoryBufferRef Buffer);
 
   /// Parse the specified bitcode buffer and merge the index into CombinedIndex.
   Error readModuleSummaryIndex(MemoryBufferRef Buffer,
-                               ModuleSummaryIndex &CombinedIndex,
-                               uint64_t ModuleId);
+                               ModuleSummaryIndex &CombinedIndex);
 
   /// Parse the module summary index out of an IR file and return the module
   /// summary index object if found, or an empty summary if not. If Path refers
   /// to an empty file and IgnoreEmptyThinLTOIndexFile is true, then
   /// this function will return nullptr.
   Expected<std::unique_ptr<ModuleSummaryIndex>>
   getModuleSummaryIndexForFile(StringRef Path,
                                bool IgnoreEmptyThinLTOIndexFile = false);

llvm/include/llvm/IR/ModuleSummaryIndex.h

 
 /// 160 bits SHA1
 using ModuleHash = std::array<uint32_t, 5>;
 
 /// Type used for iterating through the global value summary map.
 using const_gvsummary_iterator = GlobalValueSummaryMapTy::const_iterator;
 using gvsummary_iterator = GlobalValueSummaryMapTy::iterator;
 
-/// String table to hold/own module path strings, which additionally holds the
-/// module ID assigned to each module during the plugin step, as well as a hash
+/// String table to hold/own module path strings, as well as a hash
 /// of the module. The StringMap makes a copy of and owns inserted strings.
-using ModulePathStringTableTy = StringMap<std::pair<uint64_t, ModuleHash>>;
+using ModulePathStringTableTy = StringMap<ModuleHash>;
 
 /// Map of global value GUID to its summary, used to identify values defined in
 /// a particular module, and provide efficient access to their summary.
 using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;
 
 /// Map of a type GUID to type id string and summary (multimap used
 /// in case of GUID conflicts).
 using TypeIdSummaryMapTy =
public:
 
   /// Returns the first GlobalValueSummary for \p ValueGUID, asserting that
   /// there
   /// is only one if \p PerModuleIndex.
   GlobalValueSummary *getGlobalValueSummary(GlobalValue::GUID ValueGUID,
                                             bool PerModuleIndex = true) const;
 
   /// Table of modules, containing module hash and id.
-  const StringMap<std::pair<uint64_t, ModuleHash>> &modulePaths() const {
+  const StringMap<ModuleHash> &modulePaths() const {
     return ModulePathStringTable;
   }
 
   /// Table of modules, containing hash and id.
-  StringMap<std::pair<uint64_t, ModuleHash>> &modulePaths() {
-    return ModulePathStringTable;
-  }
-
-  /// Get the module ID recorded for the given module path.
-  uint64_t getModuleId(const StringRef ModPath) const {
-    return ModulePathStringTable.lookup(ModPath).first;
-  }
+  StringMap<ModuleHash> &modulePaths() { return ModulePathStringTable; }
 
   /// Get the module SHA1 hash recorded for the given module path.
   const ModuleHash &getModuleHash(const StringRef ModPath) const {
     auto It = ModulePathStringTable.find(ModPath);
     assert(It != ModulePathStringTable.end() && "Module not registered");
-    return It->second.second;
+    return It->second;
   }
 
   /// Convenience method for creating a promoted global name
   /// for the given value name of a local, and its original module's ID.
   static std::string getGlobalNameForLocal(StringRef Name, ModuleHash ModHash) {
     std::string Suffix = utostr((uint64_t(ModHash[0]) << 32) |
                                 ModHash[1]); // Take the first 64 bits
     return getGlobalNameForLocal(Name, Suffix);
static StringRef getOriginalNameBeforePromote(StringRef Name) {
     std::pair<StringRef, StringRef> Pair = Name.rsplit(".llvm.");
     return Pair.first;
   }
 
   typedef ModulePathStringTableTy::value_type ModuleInfo;
 
   /// Add a new module with the given \p Hash, mapped to the given \p
   /// ModID, and return a reference to the module.
-  ModuleInfo *addModule(StringRef ModPath, uint64_t ModId,
-                        ModuleHash Hash = ModuleHash{{0}}) {
-    return &*ModulePathStringTable.insert({ModPath, {ModId, Hash}}).first;
+  ModuleInfo *addModule(StringRef ModPath, ModuleHash Hash = ModuleHash{{0}}) {
+    return &*ModulePathStringTable.insert({ModPath, Hash}).first;
   }
 
   /// Return module entry for module with the given \p ModPath.
   ModuleInfo *getModule(StringRef ModPath) {
     auto It = ModulePathStringTable.find(ModPath);
     assert(It != ModulePathStringTable.end() && "Module not registered");
     return &*It;
   }

llvm/lib/AsmParser/LLParser.cpp

if (parseUInt32(Hash[0]) || parseToken(lltok::comma, "expected ',' here") ||
       parseUInt32(Hash[3]) || parseToken(lltok::comma, "expected ',' here") ||
       parseUInt32(Hash[4]))
     return true;
 
   if (parseToken(lltok::rparen, "expected ')' here") ||
       parseToken(lltok::rparen, "expected ')' here"))
     return true;
 
-  auto ModuleEntry = Index->addModule(Path, ID, Hash);
+  auto ModuleEntry = Index->addModule(Path, Hash);
   ModuleIdMap[ID] = ModuleEntry->first();
 
   return false;
 }
 
 /// TypeIdEntry
 ///   ::= 'typeid' ':' '(' 'name' ':' STRINGCONSTANT ',' TypeIdSummary ')'
 bool LLParser::parseTypeIdEntry(unsigned ID) {

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

class ModuleSummaryIndexBitcodeReader : public BitcodeReaderBase {
 
   /// Original source file name recorded in a bitcode record.
   std::string SourceFileName;
 
   /// The string identifier given to this module by the client, normally the
   /// path to the bitcode file.
   StringRef ModulePath;
 
-  /// For per-module summary indexes, the unique numerical identifier given to
-  /// this module by the client.
-  unsigned ModuleId;
-
   /// Callback to ask whether a symbol is the prevailing copy when invoked
   /// during combined index building.
   std::function<bool(GlobalValue::GUID)> IsPrevailing;
 
   /// Saves the stack ids from the STACK_IDS record to consult when adding stack
   /// ids from the lists in the callsite and alloc entries to the index.
   std::vector<uint64_t> StackIds;
 
 public:
   ModuleSummaryIndexBitcodeReader(
       BitstreamCursor Stream, StringRef Strtab, ModuleSummaryIndex &TheIndex,
-      StringRef ModulePath, unsigned ModuleId,
+      StringRef ModulePath,
       std::function<bool(GlobalValue::GUID)> IsPrevailing = nullptr);
 
   Error parseModule();
 
 private:
   void setValueGUID(uint64_t ValueID, StringRef ValueName,
                     GlobalValue::LinkageTypes Linkage,
                     StringRef SourceFileName);

 }
 
 std::vector<StructType *> BitcodeReader::getIdentifiedStructTypes() const {
   return IdentifiedStructTypes;
 }
 
 ModuleSummaryIndexBitcodeReader::ModuleSummaryIndexBitcodeReader(
     BitstreamCursor Cursor, StringRef Strtab, ModuleSummaryIndex &TheIndex,
-    StringRef ModulePath, unsigned ModuleId,
-    std::function<bool(GlobalValue::GUID)> IsPrevailing)
+    StringRef ModulePath, std::function<bool(GlobalValue::GUID)> IsPrevailing)
     : BitcodeReaderBase(std::move(Cursor), Strtab), TheIndex(TheIndex),
-      ModulePath(ModulePath), ModuleId(ModuleId), IsPrevailing(IsPrevailing) {}
+      ModulePath(ModulePath), IsPrevailing(IsPrevailing) {}
 
 void ModuleSummaryIndexBitcodeReader::addThisModule() {
-  TheIndex.addModule(ModulePath, ModuleId);
+  TheIndex.addModule(ModulePath);
 }
 
 ModuleSummaryIndex::ModuleInfo *
 ModuleSummaryIndexBitcodeReader::getThisModule() {
   return TheIndex.getModule(ModulePath);
 }
 
 template <bool AllowNullValueInfo>
case BitstreamEntry::Record: {
             return error("Invalid record");
           SourceFileName = ValueName.c_str();
           break;
         }
         /// MODULE_CODE_HASH: [5*i32]
         case bitc::MODULE_CODE_HASH: {
           if (Record.size() != 5)
             return error("Invalid hash length " + Twine(Record.size()).str());
-          auto &Hash = getThisModule()->second.second;
+          auto &Hash = getThisModule()->second;
           int Pos = 0;
           for (auto &Val : Record) {
             assert(!(Val >> 32) && "Unexpected high bits set");
             Hash[Pos++] = Val;
           }
           break;
         }
         /// MODULE_CODE_VSTOFFSET: [offset]
default: // Default behavior: ignore.
       break;
     case bitc::MST_CODE_ENTRY: {
       // MST_ENTRY: [modid, namechar x N]
       uint64_t ModuleId = Record[0];
 
       if (convertToString(Record, 1, ModulePath))
         return error("Invalid record");
 
-      LastSeenModule = TheIndex.addModule(ModulePath, ModuleId);
+      LastSeenModule = TheIndex.addModule(ModulePath);
       ModuleIdMap[ModuleId] = LastSeenModule->first();
 
       ModulePath.clear();
       break;
     }
     /// MST_CODE_HASH: [5*i32]
     case bitc::MST_CODE_HASH: {
       if (Record.size() != 5)
         return error("Invalid hash length " + Twine(Record.size()).str());
       if (!LastSeenModule)
         return error("Invalid hash that does not follow a module path");
       int Pos = 0;
       for (auto &Val : Record) {
         assert(!(Val >> 32) && "Unexpected high bits set");
-        LastSeenModule->second.second[Pos++] = Val;
+        LastSeenModule->second[Pos++] = Val;
       }
       // Reset LastSeenModule to avoid overriding the hash unexpectedly.
       LastSeenModule = nullptr;
       break;
     }
     }
   }
   llvm_unreachable("Exit infinite loop");
return getModuleImpl(Context, false, ShouldLazyLoadMetadata, IsImporting,
                        Callbacks);
 }
 
 // Parse the specified bitcode buffer and merge the index into CombinedIndex.
 // We don't use ModuleIdentifier here because the client may need to control the
 // module path used in the combined summary (e.g. when reading summaries for
 // regular LTO modules).
 Error BitcodeModule::readSummary(
-    ModuleSummaryIndex &CombinedIndex, StringRef ModulePath, uint64_t ModuleId,
+    ModuleSummaryIndex &CombinedIndex, StringRef ModulePath,
     std::function<bool(GlobalValue::GUID)> IsPrevailing) {
   BitstreamCursor Stream(Buffer);
   if (Error JumpFailed = Stream.JumpToBit(ModuleBit))
     return JumpFailed;
 
   ModuleSummaryIndexBitcodeReader R(std::move(Stream), Strtab, CombinedIndex,
-                                    ModulePath, ModuleId, IsPrevailing);
+                                    ModulePath, IsPrevailing);
   return R.parseModule();
 }
 
 // Parse the specified bitcode buffer, returning the function info index.
 Expected<std::unique_ptr<ModuleSummaryIndex>> BitcodeModule::getSummary() {
   BitstreamCursor Stream(Buffer);
   if (Error JumpFailed = Stream.JumpToBit(ModuleBit))
     return std::move(JumpFailed);
Expected<std::string> llvm::getBitcodeProducerString(MemoryBufferRef Buffer) {
   Expected<BitstreamCursor> StreamOrErr = initStream(Buffer);
   if (!StreamOrErr)
     return StreamOrErr.takeError();
 
   return readIdentificationCode(*StreamOrErr);
 }
 
 Error llvm::readModuleSummaryIndex(MemoryBufferRef Buffer,
-                                   ModuleSummaryIndex &CombinedIndex,
-                                   uint64_t ModuleId) {
+                                   ModuleSummaryIndex &CombinedIndex) {
   Expected<BitcodeModule> BM = getSingleModule(Buffer);
   if (!BM)
     return BM.takeError();
 
-  return BM->readSummary(CombinedIndex, BM->getModuleIdentifier(), ModuleId);
+  return BM->readSummary(CombinedIndex, BM->getModuleIdentifier());
 }
 
 Expected<std::unique_ptr<ModuleSummaryIndex>>
 llvm::getModuleSummaryIndex(MemoryBufferRef Buffer) {
   Expected<BitcodeModule> BM = getSingleModule(Buffer);
   if (!BM)
     return BM.takeError();
 

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

class IndexBitcodeWriter : public BitcodeWriterBase {
   // The sorted stack id indices actually used in the summary entries being
   // written, which will be a subset of those in the full index in the case of
   // distributed indexes.
   std::vector<unsigned> StackIdIndices;
 
   /// Tracks the last value id recorded in the GUIDToValueMap.
   unsigned GlobalValueId = 0;
 
+  /// Tracks the assignment of module paths in the module path string table to
+  /// an id assigned for use in summary references to the module path.
+  DenseMap<StringRef, uint64_t> ModuleIdMap;
+
 public:
   /// Constructs a IndexBitcodeWriter object for the given combined index,
   /// writing to the provided \p Buffer. When writing a subset of the index
   /// for a distributed backend, provide a \p ModuleToSummariesForIndex map.
   IndexBitcodeWriter(BitstreamWriter &Stream, StringTableBuilder &StrtabBuilder,
                      const ModuleSummaryIndex &Index,
                      const std::map<std::string, GVSummaryMapTy>
                          *ModuleToSummariesForIndex = nullptr)
if (ModuleToSummariesForIndex) {
           // case we shouldn't be importing (the ModuleToSummariesForIndex
           // would only include the module we are writing and index for).
           assert(ModuleToSummariesForIndex->size() == 1);
           continue;
         }
         Callback(*MPI);
       }
     } else {
-      for (const auto &MPSE : Index.modulePaths())
-        Callback(MPSE);
+      // Since StringMap iteration order isn't guaranteed, order by path string
+      // first.
+      // FIXME: Make this a vector of StringMapEntry instead to avoid the later
+      // map lookup.
+      std::vector<StringRef> ModulePaths;
+      for (auto &[ModPath, _] : Index.modulePaths())
+        ModulePaths.push_back(ModPath);
+      llvm::sort(ModulePaths.begin(), ModulePaths.end());
+      for (auto &ModPath : ModulePaths)
+        Callback(*Index.modulePaths().find(ModPath));
     }
   }
 
   /// Main entry point for writing a combined index to bitcode.
   void write();
 
 private:
   void writeModStrings();
void IndexBitcodeWriter::writeModStrings() {
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));
   unsigned AbbrevHash = Stream.EmitAbbrev(std::move(Abbv));
 
   SmallVector<unsigned, 64> Vals;
-  forEachModule(
-      [&](const StringMapEntry<std::pair<uint64_t, ModuleHash>> &MPSE) {
-        StringRef Key = MPSE.getKey();
-        const auto &Value = MPSE.getValue();
-        StringEncoding Bits = getStringEncoding(Key);
-        unsigned AbbrevToUse = Abbrev8Bit;
-        if (Bits == SE_Char6)
-          AbbrevToUse = Abbrev6Bit;
-        else if (Bits == SE_Fixed7)
-          AbbrevToUse = Abbrev7Bit;
+  forEachModule([&](const StringMapEntry<ModuleHash> &MPSE) {
+    const auto &Hash = MPSE.getValue();
 
-        Vals.push_back(Value.first);
-        Vals.append(Key.begin(), Key.end());
+    auto ModuleId = ModuleIdMap.size();
+    ModuleIdMap[Key] = ModuleId;
+    Vals.push_back(ModuleId);
 
-        // Emit the finished record.
-        Stream.EmitRecord(bitc::MST_CODE_ENTRY, Vals, AbbrevToUse);
 
-        // Emit an optional hash for the module now
-        const auto &Hash = Value.second;
-        if (llvm::any_of(Hash, [](uint32_t H) { return H; })) {
-          Vals.assign(Hash.begin(), Hash.end());
-          // Emit the hash record.
-          Stream.EmitRecord(bitc::MST_CODE_HASH, Vals, AbbrevHash);
-        }
 
-        Vals.clear();
-      });
   Stream.ExitBlock();
 }
 
 /// Write the function type metadata related records that need to appear before
 /// a function summary entry (whether per-module or combined).
 template <typename Fn>
 static void writeFunctionTypeMetadataRecords(BitstreamWriter &Stream,
                                              FunctionSummary *FS,
if (auto *AS = dyn_cast<AliasSummary>(S)) {
       // Will process aliases as a post-pass because the reader wants all
       // global to be loaded first.
       Aliases.push_back(AS);
       return;
     }
 
     if (auto *VS = dyn_cast<GlobalVarSummary>(S)) {
       NameVals.push_back(*ValueId);
-      NameVals.push_back(Index.getModuleId(VS->modulePath()));
+      assert(ModuleIdMap.count(VS->modulePath()));
+      NameVals.push_back(ModuleIdMap[VS->modulePath()]);
       NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
       NameVals.push_back(getEncodedGVarFlags(VS->varflags()));
       for (auto &RI : VS->refs()) {
         auto RefValueId = getValueId(RI.getGUID());
         if (!RefValueId)
           continue;
         NameVals.push_back(*RefValueId);
       }
writeFunctionHeapProfileRecords(
           // Get the corresponding index into the list of StackIdIndices
           // actually being written for this combined index (which may be a
           // subset in the case of distributed indexes).
           auto Lower = llvm::lower_bound(StackIdIndices, I);
           return std::distance(StackIdIndices.begin(), Lower);
         });
 
     NameVals.push_back(*ValueId);
-    NameVals.push_back(Index.getModuleId(FS->modulePath()));
+    assert(ModuleIdMap.count(FS->modulePath()));
+    NameVals.push_back(ModuleIdMap[FS->modulePath()]);
     NameVals.push_back(getEncodedGVSummaryFlags(FS->flags()));
     NameVals.push_back(FS->instCount());
     NameVals.push_back(getEncodedFFlags(FS->fflags()));
     NameVals.push_back(FS->entryCount());
 
     // Fill in below
     NameVals.push_back(0); // numrefs
     NameVals.push_back(0); // rorefcnt
forEachSummary([&](GVInfo I, bool IsAliasee) {
     NameVals.clear();
     MaybeEmitOriginalName(*S);
   });
 
   for (auto *AS : Aliases) {
     auto AliasValueId = SummaryToValueIdMap[AS];
     assert(AliasValueId);
     NameVals.push_back(AliasValueId);
-    NameVals.push_back(Index.getModuleId(AS->modulePath()));
+    assert(ModuleIdMap.count(AS->modulePath()));
+    NameVals.push_back(ModuleIdMap[AS->modulePath()]);
     NameVals.push_back(getEncodedGVSummaryFlags(AS->flags()));
     auto AliaseeValueId = SummaryToValueIdMap[&AS->getAliasee()];
     assert(AliaseeValueId);
     NameVals.push_back(AliaseeValueId);
 
     // Emit the finished record.
     Stream.EmitRecord(bitc::FS_COMBINED_ALIAS, NameVals, FSAliasAbbrev);
     NameVals.clear();

llvm/lib/IR/AsmWriter.cpp

 
 // Iterate through all the GUID in the index and create slots for them.
 int SlotTracker::processIndex() {
   ST_DEBUG("begin processIndex!\n");
   assert(TheIndex);
 
   // The first block of slots are just the module ids, which start at 0 and are
   // assigned consecutively. Since the StringMap iteration order isn't
-  // guaranteed, use a std::map to order by module ID before assigning slots.
-  std::map<uint64_t, StringRef> ModuleIdToPathMap;
-  for (auto &[ModPath, ModId] : TheIndex->modulePaths())
-    ModuleIdToPathMap[ModId.first] = ModPath;
-  for (auto &ModPair : ModuleIdToPathMap)
-    CreateModulePathSlot(ModPair.second);
+  // guaranteed, order by path string before assigning slots.
+  std::vector<StringRef> ModulePaths;
+  for (auto &[ModPath, _] : TheIndex->modulePaths())
+    ModulePaths.push_back(ModPath);
+  llvm::sort(ModulePaths.begin(), ModulePaths.end());
+  for (auto &ModPath : ModulePaths)
+    CreateModulePathSlot(ModPath);
 
   // Start numbering the GUIDs after the module ids.
   GUIDNext = ModulePathNext;
 
   for (auto &GlobalList : *TheIndex)
     CreateGUIDSlot(GlobalList.first);
 
   for (auto &TId : TheIndex->typeIdCompatibleVtableMap())
void AssemblyWriter::printModuleSummaryIndex() {
   Out << "\n";
 
   // Print module path entries. To print in order, add paths to a vector
   // indexed by module slot.
   std::vector<std::pair<std::string, ModuleHash>> moduleVec;
   std::string RegularLTOModuleName =
       ModuleSummaryIndex::getRegularLTOModuleName();
   moduleVec.resize(TheIndex->modulePaths().size());
-  for (auto &[ModPath, ModId] : TheIndex->modulePaths())
+  for (auto &[ModPath, ModHash] : TheIndex->modulePaths())
     moduleVec[Machine.getModulePathSlot(ModPath)] = std::make_pair(
-        // A module id of -1 is a special entry for a regular LTO module created
-        // during the thin link.
-        ModId.first == -1u ? RegularLTOModuleName : std::string(ModPath),
-        ModId.second);
+        // An empty module path is a special entry for a regular LTO module
+        // created during the thin link.
+        ModPath.empty() ? RegularLTOModuleName : std::string(ModPath), ModHash);
 
   unsigned i = 0;
   for (auto &ModPair : moduleVec) {
     Out << "^" << i++ << " = module: (";
     Out << "path: \"";
     printEscapedString(ModPair.first, Out);
     Out << "\", hash: (";
     FieldSeparator FS;

llvm/lib/IR/ModuleSummaryIndex.cpp

void ModuleSummaryIndex::exportToDot(
     raw_ostream &OS,
     const DenseSet<GlobalValue::GUID> &GUIDPreservedSymbols) const {
   std::vector<Edge> CrossModuleEdges;
   DenseMap<GlobalValue::GUID, std::vector<uint64_t>> NodeMap;
   using GVSOrderedMapTy = std::map<GlobalValue::GUID, GlobalValueSummary *>;
   std::map<StringRef, GVSOrderedMapTy> ModuleToDefinedGVS;
   collectDefinedGVSummariesPerModule(ModuleToDefinedGVS);
 
+  // Assign an id to each module path for use in graph labels. Since the
+  // StringMap iteration order isn't guaranteed, order by path string before
+  // assigning ids.
+  std::vector<StringRef> ModulePaths;
+  for (auto &[ModPath, _] : modulePaths())
+    ModulePaths.push_back(ModPath);
+  llvm::sort(ModulePaths);
+  DenseMap<StringRef, uint64_t> ModuleIdMap;
+  for (auto &ModPath : ModulePaths)
+    ModuleIdMap.try_emplace(ModPath, ModuleIdMap.size());
+
   // Get node identifier in form MXXX_<GUID>. The MXXX prefix is required,
   // because we may have multiple linkonce functions summaries.
   auto NodeId = [](uint64_t ModId, GlobalValue::GUID Id) {
     return ModId == (uint64_t)-1 ? std::to_string(Id)
                                  : std::string("M") + std::to_string(ModId) +
                                        "_" + std::to_string(Id);
   };
 
auto DrawEdge = [&](const char *Pfx, uint64_t SrcMod, GlobalValue::GUID SrcId,
 
     assert(static_cast<size_t>(TypeOrHotness) < std::size(EdgeAttrs));
     OS << Pfx << NodeId(SrcMod, SrcId) << " -> " << NodeId(DstMod, DstId)
        << EdgeAttrs[TypeOrHotness] << "\n";
   };
 
   OS << "digraph Summary {\n";
   for (auto &ModIt : ModuleToDefinedGVS) {
-    auto ModId = getModuleId(ModIt.first);
+    // Will be empty for a just built per-module index, which doesn't setup a
+    // module paths table. In that case use 0 as the module id.
+    assert(ModuleIdMap.count(ModIt.first) || ModuleIdMap.empty());
+    auto ModId = ModuleIdMap.empty() ? 0 : ModuleIdMap[ModIt.first];
     OS << "  // Module: " << ModIt.first << "\n";
     OS << "  subgraph cluster_" << std::to_string(ModId) << " {\n";
     OS << "    style = filled;\n";
     OS << "    color = lightgrey;\n";
     OS << "    label = \"" << sys::path::filename(ModIt.first) << "\";\n";
     OS << "    node [style=filled,fillcolor=lightblue];\n";
 
     auto &GVSMap = ModIt.second;

llvm/lib/IRPrinter/IRPrintingPasses.cpp

if (llvm::isFunctionInPrintList("*")) {
     }
   }
 
   ModuleSummaryIndex *Index =
       EmitSummaryIndex ? &(AM.getResult<ModuleSummaryIndexAnalysis>(M))
                        : nullptr;
   if (Index) {
     if (Index->modulePaths().empty())
-      Index->addModule("", 0);
+      Index->addModule("");
     Index->print(OS);
   }
 
   return PreservedAnalyses::all();
 }
 
 PrintFunctionPass::PrintFunctionPass() : OS(dbgs()) {}
 PrintFunctionPass::PrintFunctionPass(raw_ostream &OS, const std::string &Banner)

llvm/lib/LTO/LTO.cpp

struct ImportModule {
     ImportMapIteratorTy ModIt;
     const ModuleSummaryIndex::ModuleInfo *ModInfo;
 
     StringRef getIdentifier() const { return ModIt->getFirst(); }
     const FunctionImporter::FunctionsToImportTy &getFunctions() const {
       return ModIt->second;
     }
 
-    const ModuleHash &getHash() const { return ModInfo->second.second; }
+    const ModuleHash &getHash() const { return ModInfo->second; }
   };
 
   std::vector<ImportModule> ImportModulesVector;
   ImportModulesVector.reserve(ImportList.size());
 
   for (ImportMapIteratorTy It = ImportList.begin(); It != ImportList.end();
        ++It) {
     ImportModulesVector.push_back({It, Index.getModule(It->getFirst())});
Error LTO::addModule(InputFile &Input, unsigned ModI,
   if (!ModOrErr)
     return ModOrErr.takeError();
 
   if (!LTOInfo->HasSummary)
     return linkRegularLTO(std::move(*ModOrErr), /*LivenessFromIndex=*/false);
 
   // Regular LTO module summaries are added to a dummy module that represents
   // the combined regular LTO module.
-  if (Error Err = BM.readSummary(ThinLTO.CombinedIndex, "", -1ull))
+  if (Error Err = BM.readSummary(ThinLTO.CombinedIndex, ""))
     return Err;
   RegularLTO.ModsWithSummaries.push_back(std::move(*ModOrErr));
   return Error::success();
 }
 
 // Checks whether the given global value is in a non-prevailing comdat
 // (comdat containing values the linker indicated were not prevailing,
 // which we then dropped to available_externally), and if so, removes
for (const InputFile::Symbol &Sym : Syms) {
     if (!Sym.getIRName().empty()) {
       auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
           Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
       if (Res.Prevailing)
         ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
     }
   }
 
-  uint64_t ModuleId = ThinLTO.ModuleMap.size();
   if (Error Err =
           BM.readSummary(ThinLTO.CombinedIndex, BM.getModuleIdentifier(),
-                         ModuleId, [&](GlobalValue::GUID GUID) {
+                         [&](GlobalValue::GUID GUID) {
                            return ThinLTO.PrevailingModuleForGUID[GUID] ==
                                   BM.getModuleIdentifier();
                          }))
     return Err;
-  LLVM_DEBUG(dbgs() << "Module " << ModuleId << ": " << BM.getModuleIdentifier()
-                    << "\n");
+  LLVM_DEBUG(dbgs() << "Module " << BM.getModuleIdentifier() << "\n");
 
   for (const InputFile::Symbol &Sym : Syms) {
     assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
 
     if (!Sym.getIRName().empty()) {
       auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
           Sym.getIRName(), GlobalValue::ExternalLinkage, ""));

llvm/lib/LTO/ThinLTOCodeGenerator.cpp

 
 /**
  * Produce the combined summary index from all the bitcode files:
  * "thin-link".
  */
 std::unique_ptr<ModuleSummaryIndex> ThinLTOCodeGenerator::linkCombinedIndex() {
   std::unique_ptr<ModuleSummaryIndex> CombinedIndex =
       std::make_unique<ModuleSummaryIndex>(/*HaveGVs=*/false);
-  uint64_t NextModuleId = 0;
   for (auto &Mod : Modules) {
     auto &M = Mod->getSingleBitcodeModule();
-    if (Error Err =
-            M.readSummary(*CombinedIndex, Mod->getName(), NextModuleId++)) {
+    if (Error Err = M.readSummary(*CombinedIndex, Mod->getName())) {
       // FIXME diagnose
       logAllUnhandledErrors(
           std::move(Err), errs(),
           "error: can't create module summary index for buffer: ");
       return nullptr;
     }
   }
   return CombinedIndex;

llvm/test/Assembler/thinlto-multiple-summaries-for-guid.ll

 ; Checks that llvm-as/llvm-dis correctly assembles/disassembles index with
 ; multiple summaries with single GUID.
 ; RUN: llvm-as %s -o - | llvm-dis -o - | FileCheck %s
 
 source_filename = "index.bc"
 
-^0 = module: (path: "main.bc", hash: (3499594384, 1671013073, 3271036935, 1830411232, 59290952))
-; CHECK: ^0 = module: (path: "main.bc", hash: (3499594384, 1671013073, 3271036935, 1830411232, 59290952))
-^1 = module: (path: "[Regular LTO]", hash: (0, 0, 0, 0, 0))
-; CHECK-NEXT: ^1 = module: (path: "[Regular LTO]", hash: (0, 0, 0, 0, 0))
-^2 = gv: (guid: 13351721993301222997, summaries: (function: (module: ^0, flags: (linkage: linkonce_odr, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 1), insts: 1), function: (module: ^1, flags: (linkage: available_externally, notEligibleToImport: 1, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 1)))
-; CHECK-NEXT: ^2 = gv: (guid: 13351721993301222997, summaries: (function: (module: ^0, flags: (linkage: linkonce_odr, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 1), insts: 1), function: (module: ^1, flags: (linkage: available_externally, visibility: default, notEligibleToImport: 1, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 1)))
+^0 = module: (path: "[Regular LTO]", hash: (0, 0, 0, 0, 0))
+; CHECK: ^0 = module: (path: "[Regular LTO]", hash: (0, 0, 0, 0, 0))
+^1 = module: (path: "main.bc", hash: (3499594384, 1671013073, 3271036935, 1830411232, 59290952))
+; CHECK-NEXT: ^1 = module: (path: "main.bc", hash: (3499594384, 1671013073, 3271036935, 1830411232, 59290952))
+^2 = gv: (guid: 13351721993301222997, summaries: (function: (module: ^1, flags: (linkage: linkonce_odr, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 1), insts: 1), function: (module: ^1, flags: (linkage: available_externally, notEligibleToImport: 1, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 1)))
+; CHECK-NEXT: ^2 = gv: (guid: 13351721993301222997, summaries: (function: (module: ^1, flags: (linkage: linkonce_odr, visibility: default, notEligibleToImport: 0, live: 1, dsoLocal: 1, canAutoHide: 1), insts: 1), function: (module: ^1, flags: (linkage: available_externally, visibility: default, notEligibleToImport: 1, live: 1, dsoLocal: 1, canAutoHide: 0), insts: 1)))

llvm/test/ThinLTO/X86/dot-dumper-full-lto.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/dot-dumper.ll -o %t2.bc
 ; RUN: llvm-lto2 run -save-temps %t1.bc %t2.bc -o %t3 \
 ; RUN:  -r=%t1.bc,main,px \
 ; RUN:  -r=%t1.bc,A, \
 ; RUN:  -r=%t2.bc,foo,p \
 ; RUN:  -r=%t2.bc,bar,p \
 ; RUN:  -r=%t2.bc,A,p \
 ; RUN:  -r=%t2.bc,B,p
 ; RUN: cat %t3.index.dot | FileCheck %s
 
-; CHECK: subgraph cluster_4294967295
-; CHECK:   M4294967295_[[ID:[0-9]+]]{{.*}}main
+; CHECK: subgraph cluster_0
+; CHECK:   M0_[[ID:[0-9]+]]{{.*}}main
 ; CHECK: // Cross-module edges:
-; CHECK:  M4294967295_[[ID]] -> M0_{{[0-9]+}}{{.*}}// ref
+; CHECK:  M0_[[ID]] -> M1_{{[0-9]+}}{{.*}}// ref
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @A = external global i32
 
 define i32 @main() {
   %v = load i32, ptr @A
   ret i32 %v
 }
 
 !0 = !{i32 1, !"ThinLTO", i32 0}
 !llvm.module.flags = !{ !0 }

llvm/tools/llvm-lto/llvm-lto.cpp

 }
 
 /// Create a combined index file from the input IR files and write it.
 ///
 /// This is meant to enable testing of ThinLTO combined index generation,
 /// currently available via the gold plugin via -thinlto.
 static void createCombinedModuleSummaryIndex() {
   ModuleSummaryIndex CombinedIndex(/*HaveGVs=*/false);
-  uint64_t NextModuleId = 0;
   for (auto &Filename : InputFilenames) {
     ExitOnError ExitOnErr("llvm-lto: error loading file '" + Filename + "': ");
     std::unique_ptr<MemoryBuffer> MB =
         ExitOnErr(errorOrToExpected(MemoryBuffer::getFileOrSTDIN(Filename)));
-    ExitOnErr(readModuleSummaryIndex(*MB, CombinedIndex, NextModuleId++));
+    ExitOnErr(readModuleSummaryIndex(*MB, CombinedIndex));
   }
   // In order to use this index for testing, specifically import testing, we
   // need to update any indirect call edges created from SamplePGO, so that they
   // point to the correct GUIDs.
   updateIndirectCalls(CombinedIndex);
   std::error_code EC;
   assert(!OutputFilename.empty());
   raw_fd_ostream OS(OutputFilename + ".thinlto.bc", EC,