[PATCH] D71437: [PDB] Print the most redundant type record indices with /summary

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 4 inline comments as done.
rnk added a subscriber: bkramer.
rnk added inline comments.


================
Comment at: lld/COFF/PDB.cpp:431
+    // collecting statistics.
+    tpiCounts.resize(tMerger.getTypeTable().size());
+    ipiCounts.resize(tMerger.getIDTable().size());
----------------
MaskRay wrote:
> Can `tpiCounts` be a non-empty vector before `resize()`?
> 
> If yes, `assign(tMerger.getTypeTable().size(), 0);`
Yes, it can be non-empty, but why is assign better? `resize()` does zero initialization, it is not reserve.


================
Comment at: lld/COFF/PDB.cpp:437
+      if (dstIdx.isSimple())
+        continue; // Untranslated or other.
+      SmallVectorImpl<uint32_t> &counts =
----------------
MaskRay wrote:
> Excuse my ignorance. What does "Untranslated or other" mean? I cannot find "untranslated" at other call sites of `isSimple`.
Type merging can fail for some records, and rather than failing the whole link, the the record is discarded. Any indexes referring to it are replaced with `SimpleTypeKind::NotTranslated`, which is a simple type. The programmer could reasonably assume that complex types always map to complex types, but this is the common case where that assumption won't hold. The `toArrayIndex` call below will assert in that case.


================
Comment at: lld/COFF/PDB.cpp:1390
+    };
+    SmallVector<TypeSizeInfo, 0> tsis;
+    for (auto e : enumerate(recCounts)) {
----------------
MaskRay wrote:
> I've seen `SmallVector<..., 0>` in some code. Does that has any advantage over `std::vector<...>`?
I've been assuming it does since I saw @bkramer doing it:
https://github.com/llvm/llvm-project/commit/bb76eaa2c73818c79773fdc8012b92846a1eebbe


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71437/new/

https://reviews.llvm.org/D71437