[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">;
----------------
aganea wrote:
> 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()`.
@MaskRay I feel both 0 and -1 are not user-friendly. I think it is better to say what we mean, `--threads=no` or `--threads=default`. But to follow the principle of least astonishment, `--threads=0` or `--threads=1` could disable multi-threading. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76885