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

Tue Oct 13 19:05:59 PDT 2020

tejohnson added a comment.

In D87966#2329086 <https://reviews.llvm.org/D87966#2329086>, @aganea wrote:

> In D87966#2328818 <https://reviews.llvm.org/D87966#2328818>, @tejohnson wrote:
>
>> In D87966#2328736 <https://reviews.llvm.org/D87966#2328736>, @aganea wrote:
>>
>>> @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) { ... }`.
>>
>> Great, thanks! LGTM with request for comment as described below.
>>
>>> This test also implictly covers the "InProcessThinBackend" codepath with `/opt:lldltojobs=1` which I don't see how to cover explicitly otherwise.
>>
>> I don't see how this test covers that code path since it isn't using in process backends, but I'm not really sure how to test this case effectively - like the original patch, it is just a performance change on the in process backends case.
>
> I meant if we have:
> `; RUN: lld-link /lldsavetemps /out:%t4.exe /entry:main /subsystem:console %t2.obj %t1.obj /opt:lldltojobs=1`
> We end up in `if (BackendProc->getThreadCount() == 1)` as if we did:
> `RUN: lld-link -thinlto-index-only:%t3 /entry:main %t1.obj %t2.obj`

Oh, I see what you are saying. Makes sense. New comment looks good, thanks.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87966