[PATCH] D87966: [ThinLTO] Re-order modules for optimal multi-threaded processing

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

@tejohnson I added the test in LLD, since the gold tests only run on Linux, which is harder for me to test & debug. The test fails when the following block is removed: `if (BackendProc->getThreadCount() == 1) { ... }`.
This test also implictly covers the "InProcessThinBackend" codepath with `/opt:lldltojobs=1` with which I don't see how to cover explicitly otherwise.



================
Comment at: llvm/lib/LTO/LTO.cpp:1468
 
+  if (RegularLTO.ParallelCodeGenParallelismLevel == 1 ||
+      BackendProc->kind() == ThinBackendProc::WriteIndexesKind) {
----------------
tejohnson wrote:
> The ParallelCodeGenParallelismLevel is unrelated to the number of threads we will be using for ThinLTO backends (rather it is related to regular LTO code generation). For the latter you would need to do something like call getThreadCount() on the BackendThreadPool on the InProcessThinBackend.
Fixed by adding a `virtual ThinBackendProc::getThreadCount()` API.


================
Comment at: llvm/lib/LTO/LTO.cpp:1469
+  if (RegularLTO.ParallelCodeGenParallelismLevel == 1 ||
+      BackendProc->kind() == ThinBackendProc::WriteIndexesKind) {
+    // Process the modules in the order they were provided.
----------------
tejohnson wrote:
> Needs comment as to why we are not doing this for WriteIndexesKind
With the new `ThinBackendProc::getThreadCount()` things are a bit more explicit I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87966