[PDB] Use one func id DenseMap instead of per-source maps, NFC

[rnk]

Summary

This avoids some DenseMap copies when /Zi is in use, and results in
fewer data structures.

[aganea]

Looks good, thanks!

lld/COFF/DebugTypes.cpp

661

Before, we were calling this if, even for non-uniqued records, meaning that we were doing (potentially) too many insertions. Now this happens only for 'unique' records, do I understand that well?

[rnk]

lld/COFF/DebugTypes.cpp

661

Yes. Previously, each source had its own map, which needed to to contain all LF_FUNC_ID records even if they were not unique. Now, because we later merge together all the mappings from all the sources, we have one map that is a complete global mapping from func id to type for the final PDB.

lld/COFF/DebugTypes.h

public:
 
   // Remap a type index in place.
   bool remapTypeIndex(TypeIndex &ti, llvm::codeview::TiRefKind refKind) const;
 
 protected:
   void remapRecord(MutableArrayRef<uint8_t> rec,
                    ArrayRef<llvm::codeview::TiReference> typeRefs);
 
-  void mergeTypeRecord(llvm::codeview::CVType ty);
+  void mergeTypeRecord(TypeIndex curIndex, llvm::codeview::CVType ty);
 
   // Merge the type records listed in uniqueTypes. beginIndex is the TypeIndex
   // of the first record in this source, typically 0x1000. When PCHs are
   // involved, it may start higher.
   void mergeUniqueTypeRecords(
       ArrayRef<uint8_t> debugTypes,
       TypeIndex beginIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex));
 
public:
   llvm::ArrayRef<TypeIndex> ipiMap;
 
   /// Array of global type hashes, indexed by TypeIndex. May be calculated on
   /// demand, or present in input object files.
   llvm::ArrayRef<llvm::codeview::GloballyHashedType> ghashes;
 
   /// When ghashing is used, record the mapping from LF_[M]FUNC_ID to function
   /// type index here. Both indices are PDB indices, not object type indexes.
-  llvm::DenseMap<TypeIndex, TypeIndex> funcIdToType;
+  std::vector<std::pair<TypeIndex, TypeIndex>> funcIdToType;
 
   /// Indicates if a type record is an item index or a type index.
   llvm::BitVector isItemIndex;
 
   /// A list of all "unique" type indices which must be merged into the final
   /// PDB. GHash type deduplication produces this list, and it should be
   /// considerably smaller than the input.
   std::vector<uint32_t> uniqueTypes;

lld/COFF/DebugTypes.cpp

Error UsePrecompSource::mergeInPrecompHeaderObj() {
   assert(precompDependency.getStartTypeIndex() ==
          TypeIndex::FirstNonSimpleIndex);
   assert(precompDependency.getTypesCount() <= precompSrc->tpiMap.size());
   // Use the previously remapped index map from the precompiled headers.
   indexMapStorage.append(precompSrc->tpiMap.begin(),
                          precompSrc->tpiMap.begin() +
                              precompDependency.getTypesCount());
 
-  if (config->debugGHashes)
-    funcIdToType = precompSrc->funcIdToType; // FIXME: Save copy
-
   return Error::success();
 }
 
 Error UsePrecompSource::mergeDebugT(TypeMerger *m) {
   // This object was compiled with /Yu, so process the corresponding
   // precompiled headers object (/Yc) first. Some type indices in the current
   // object are referencing data in the precompiled headers object, so we need
   // both to be loaded.
void TpiSource::fillIsItemIndexFromDebugT() {
   isItemIndex.resize(ghashes.size());
   forEachTypeChecked(file->debugTypes, [&](const CVType &ty) {
     if (isIdRecord(ty.kind()))
       isItemIndex.set(index);
     ++index;
   });
 }
 
-void TpiSource::mergeTypeRecord(CVType ty) {
+void TpiSource::mergeTypeRecord(TypeIndex curIndex, CVType ty) {
   // Decide if the merged type goes into TPI or IPI.
   bool isItem = isIdRecord(ty.kind());
   MergedInfo &merged = isItem ? mergedIpi : mergedTpi;
 
   // Copy the type into our mutable buffer.
   assert(ty.length() <= codeview::MaxRecordLength);
   size_t offset = merged.recs.size();
   size_t newSize = alignTo(ty.length(), 4);
   merged.recs.resize(offset + newSize);
   auto newRec = makeMutableArrayRef(&merged.recs[offset], newSize);
   memcpy(newRec.data(), ty.data().data(), newSize);
 
   // Fix up the record prefix and padding bytes if it required resizing.
   if (newSize != ty.length()) {
     reinterpret_cast<RecordPrefix *>(newRec.data())->RecordLen = newSize - 2;
     for (size_t i = ty.length(); i < newSize; ++i)
       newRec[i] = LF_PAD0 + (newSize - i);
   }
 
   // Remap the type indices in the new record.
   remapTypesInTypeRecord(newRec);
   uint32_t pdbHash = check(pdb::hashTypeRecord(CVType(newRec)));
   merged.recSizes.push_back(static_cast<uint16_t>(newSize));
   merged.recHashes.push_back(pdbHash);
+
+  // Retain a mapping from PDB function id to PDB function type. This mapping is
+  // used during symbol procesing to rewrite S_GPROC32_ID symbols to S_GPROC32
+  // symbols.
+  if (ty.kind() == LF_FUNC_ID || ty.kind() == LF_MFUNC_ID) {
+    bool success = ty.length() >= 12;
+    TypeIndex funcId = curIndex;
+    if (success)
+      success &= remapTypeIndex(funcId, TiRefKind::IndexRef);
+    TypeIndex funcType =
+        *reinterpret_cast<const TypeIndex *>(&newRec.data()[8]);
+    if (success) {
+      funcIdToType.push_back({funcId, funcType});
+    } else {
+      StringRef fname = file ? file->getName() : "<unknown PDB>";
+      warn("corrupt LF_[M]FUNC_ID record 0x" + utohexstr(curIndex.getIndex()) +
+           " in " + fname);
+    }
+  }
 }
 
 void TpiSource::mergeUniqueTypeRecords(ArrayRef<uint8_t> typeRecords,
                                        TypeIndex beginIndex) {
   // Re-sort the list of unique types by index.
   if (kind == PDB)
     assert(std::is_sorted(uniqueTypes.begin(), uniqueTypes.end()));
   else
     llvm::sort(uniqueTypes);
 
   // Accumulate all the unique types into one buffer in mergedTypes.
   uint32_t ghashIndex = 0;
   auto nextUniqueIndex = uniqueTypes.begin();
   assert(mergedTpi.recs.empty());
   assert(mergedIpi.recs.empty());
   forEachTypeChecked(typeRecords, [&](const CVType &ty) {
     if (nextUniqueIndex != uniqueTypes.end() &&
         *nextUniqueIndex == ghashIndex) {
-      mergeTypeRecord(ty);
+      mergeTypeRecord(beginIndex + ghashIndex, ty);
       ++nextUniqueIndex;
     }
-    if (ty.kind() == LF_FUNC_ID || ty.kind() == LF_MFUNC_ID) {
-      bool success = ty.length() >= 12;
-      TypeIndex srcFuncIdIndex = beginIndex + ghashIndex;
-      TypeIndex funcId = srcFuncIdIndex;
-      TypeIndex funcType;
-      if (success) {
-        funcType = *reinterpret_cast<const TypeIndex *>(&ty.data()[8]);
-        success &= remapTypeIndex(funcId, TiRefKind::IndexRef);
-        success &= remapTypeIndex(funcType, TiRefKind::TypeRef);
-      }
-      if (success) {
-        funcIdToType.insert({funcId, funcType});
-      } else {
-        StringRef fname = file ? file->getName() : "<unknown PDB>";
-        warn("corrupt LF_[M]FUNC_ID record 0x" +
-             utohexstr(srcFuncIdIndex.getIndex()) + " in " + fname);
-      }
-    }
     ++ghashIndex;
   });
   assert(nextUniqueIndex == uniqueTypes.end() &&
          "failed to merge all desired records");
   assert(uniqueTypes.size() ==
              mergedTpi.recSizes.size() + mergedIpi.recSizes.size() &&
          "missing desired record");
 }
void TypeServerSource::remapTpiWithGHashes(GHashState *g) {
   if (pdbFile.hasPDBIpiStream()) {
     pdb::TpiStream &ipi = check(pdbFile.getPDBIpiStream());
     ipiSrc->indexMapStorage.resize(ipiSrc->ghashes.size());
     ipiSrc->fillMapFromGHashes(g, ipiSrc->indexMapStorage);
     ipiMap = ipiSrc->indexMapStorage;
     ipiSrc->tpiMap = tpiMap;
     ipiSrc->ipiMap = ipiMap;
     ipiSrc->mergeUniqueTypeRecords(typeArrayToBytes(ipi.typeArray()));
-    funcIdToType = ipiSrc->funcIdToType; // FIXME: Save copy
   }
 }
 
 void UseTypeServerSource::remapTpiWithGHashes(GHashState *g) {
   // No remapping to do with /Zi objects. Simply use the index map from the type
   // server. Errors should have been reported earlier. Symbols from this object
   // will be ignored.
   Expected<TypeServerSource *> maybeTsSrc = getTypeServerSource();
   if (!maybeTsSrc) {
     typeMergingError =
         joinErrors(std::move(typeMergingError), maybeTsSrc.takeError());
     return;
   }
   TypeServerSource *tsSrc = *maybeTsSrc;
   tpiMap = tsSrc->tpiMap;
   ipiMap = tsSrc->ipiMap;
-  funcIdToType = tsSrc->funcIdToType; // FIXME: Save copy
 }
 
 void PrecompSource::loadGHashes() {
   if (getDebugH(file)) {
     warn("ignoring .debug$H section; pch with ghash is not implemented");
   }
 
   uint32_t ghashIdx = 0;
void TypeMerger::mergeTypesWithGHash() {
   // In parallel, remap all types.
   for_each(TpiSource::dependencySources, [&](TpiSource *source) {
     source->remapTpiWithGHashes(&ghashState);
   });
   parallelForEach(TpiSource::objectSources, [&](TpiSource *source) {
     source->remapTpiWithGHashes(&ghashState);
   });
 
+  // Build a global map of from function ID to function type.
+  for (TpiSource *source : TpiSource::instances) {
+    for (auto idToType : source->funcIdToType)
+      funcIdToType.insert(idToType);
+    source->funcIdToType.clear();
+  }
+
   TpiSource::clearGHashes();
 }
 
 /// Given the index into the ghash table for a particular type, return the type
 /// index for that type in the output PDB.
 static TypeIndex loadPdbTypeIndexFromCell(GHashState *g,
                                           uint32_t ghashCellIdx) {
   GHashCell cell = g->table.table[ghashCellIdx];

lld/COFF/PDB.cpp

if (kind == SymbolKind::S_GPROC32_ID || kind == SymbolKind::S_LPROC32_ID) {
     TypeIndex *ti =
         reinterpret_cast<TypeIndex *>(content.data() + refs[0].Offset);
     // `ti` is the index of a FuncIdRecord or MemberFuncIdRecord which lives in
     // the IPI stream, whose `FunctionType` member refers to the TPI stream.
     // Note that LF_FUNC_ID and LF_MFUNC_ID have the same record layout, and
     // in both cases we just need the second type index.
     if (!ti->isSimple() && !ti->isNoneType()) {
       if (config->debugGHashes) {
-        auto idToType = source->funcIdToType.find(*ti);
-        if (idToType == source->funcIdToType.end()) {
+        auto idToType = tMerger.funcIdToType.find(*ti);
+        if (idToType == tMerger.funcIdToType.end()) {
           warn(formatv("S_[GL]PROC32_ID record in {0} refers to PDB item "
                        "index {1:X} which is not a LF_[M]FUNC_ID record",
                        source->file->getName(), ti->getIndex()));
           *ti = TypeIndex(SimpleTypeKind::NotTranslated);
         } else {
           *ti = idToType->second;
         }
       } else {

lld/COFF/TypeMerger.h

inline llvm::codeview::TypeCollection &getIDTable() {
     assert(!config->debugGHashes);
     return idTable;
   }
 
   /// Use global hashes to eliminate duplicate types and identify unique type
   /// indices in each TpiSource.
   void mergeTypesWithGHash();
 
+  /// Map from PDB function id type indexes to PDB function type indexes.
+  /// Populated after mergeTypesWithGHash.
+  llvm::DenseMap<TypeIndex, TypeIndex> funcIdToType;
+
   /// Type records that will go into the PDB TPI stream.
   llvm::codeview::MergingTypeTableBuilder typeTable;
 
   /// Item records that will go into the PDB IPI stream.
   llvm::codeview::MergingTypeTableBuilder idTable;
 
   // When showSummary is enabled, these are histograms of TPI and IPI records
   // keyed by type index.
   SmallVector<uint32_t, 0> tpiCounts;
   SmallVector<uint32_t, 0> ipiCounts;
 };
 
 } // namespace coff
 } // namespace lld
 
 #endif