[PATCH] D76885: [lld][ELF][WebAssembly] Replace --(no-)threads with --threads=N

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: lld/wasm/Options.td:98
 
-def threads: F<"threads">, HelpText<"Run the linker multi-threaded">;
+defm threads : Eq<"threads", "Number of threads. 0 (default) means all of "
+                             "concurrent threads supported">;
----------------
MaskRay wrote:
> grimar wrote:
> > I'd probably expect to see a behavior when with `--threads==0` we either report error or just disable multithreading (i.e. `threads==0` ==  `threads==1`)? Perhaps the default could be `-1`.
> @aganea There is a debate about 0 vs -1. What's your opinion?
@grimar There's a subtlety there. If the code uses the `std::heavyweight_hardware_concurrency()` strategy, that means we should create a single `std::thread` per hardware **core**. That function is used in ThinLTO for example. 

Not having `--threads=...` on the cmd-line, or `--threads=0`, lets the code decide (either `std::heavyweight_hardware_concurrency()` or `std::hardware_concurrency()`)

`--threads=N` or `--threads=all` will always fallback to plain `std::hardware_concurrency()` strategy, which means to use all hardware **threads** (SMT or CMT). Typically, this mode is used when linking a large monolithic program.

Perhaps we could change to `--threads=[no|0|1]` to disable multi-threading, and let the task execute on the main thread (for debugging). Please see the definition for `llvm::get_threadpool_strategy()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76885