[PATCH] D87805: [PDB] Merge types in parallel when using ghashing

[Alexandre Ganea via Phabricator via llvm-commits]

aganea accepted this revision.
aganea added a comment.
This revision is now accepted and ready to land.

Thanks again!

LGTM with a few minor things (please see inlines below).

In D87805#2299073 <https://reviews.llvm.org/D87805#2299073>, @rnk wrote:

> For the MSVC configuration, what debug info flags are used for the compile? /Zi or /Z7, and is /Yu used?

/Z7 because of the distribution and caching, and /Yc /Yu for locally-compiled files. We also link with a bunch of third-party libs (not ours) that are built with /Zi.



================
Comment at: lld/COFF/DebugTypes.cpp:173
+  // Order dependencies first, but preserve the existing order.
+  std::vector<TpiSource *> deps;
+  std::vector<TpiSource *> objs;
----------------
Isn't all this equivalent to `llvm::stable_sort(instances, [](auto *a, auto *b) { return a->isDependency() < b->isDependency(); });`?


================
Comment at: lld/COFF/DebugTypes.cpp:335
     m->ipiCounts.resize(m->getIDTable().size());
     uint32_t srcIdx = 0;
     for (CVType &ty : types) {
----------------
Just fyi, there's a bug here when dealing with PCH (it was there since rG54a335a2f60b0f7bb85d01780bb6bbf653b1f399).
L318 should be added with:  `unsigned nbHeadRecords = indexMapStorage.size();`
L335 should be: `uint32_t srcIdx = nbHeadRecords;`


================
Comment at: lld/COFF/PDB.cpp:1575
 }
+Number);
+}
----------------
Remove this line and the one after.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87805