[PATCH] D147493: [ELF] Cap parallel::strategy to 16 threads when --threads= is unspecified

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Driver.cpp:1434
     parallel::strategy = hardware_concurrency(threads);
     config->thinLTOJobs = v;
+  } else if (parallel::strategy.compute_thread_count() > 16) {
----------------
andrewng wrote:
> This is the "default" value that I was referring to. I wasn't suggesting that this should be capped. But assuming that this is just passed on to `llvm::heavyweight_hardware_concurrency()`, then it should be fine as is because `llvm::heavyweight_hardware_concurrency()` should sort out any CPU affinity restrictions.
> 
> AFAIK, the "heavyweight" version only counts "real" cores and ignores any SMT ones. So on most X86 CPUs it will be half as many threads. Now that some X86 CPUs have a mix of fast and low-power cores, I'm not too sure what "heavyweight" is or should be in these scenarios.
> 
Indeed. I think it's worth making a https://discourse.llvm.org/ post to seek feedback whether we should take into account of hyperthreading.

In Bazel, we use distributed ThinLTO, and this value is specific to in-process ThinLTO, so this change will not affect Bazel.
I personally very rarely use `-DLLVM_ENABLE_LTO=Thin` (in-process ThinLTO), so I do not have experience about a sensible default...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147493