[PATCH] D76885: [lld][COFF][ELF][WebAssembly] Replace --[no-]threads /threads[:no] with --threads={1,2,...} /threads:{1,2,...}

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

In D76885#2194339 <https://reviews.llvm.org/D76885#2194339>, @MaskRay wrote:

> In D76885#2194272 <https://reviews.llvm.org/D76885#2194272>, @dmajor wrote:
>
>>> --no-threads is used rarely. So just delete --no-threads instead of keeping it for compatibility for a while.
>>
>> I encountered a frustrating issue with seemingly-unrelated symptoms that eventually traced back to this change.
>>
>> The wasi-sdk project was building libcxx, and its build system had set `CMAKE_EXE_LINKER_FLAGS="-Wl,--no-threads"` (I don't know why, wasi-sdk is an unfamiliar project to me, I just needed to build it for something else). When cmake was doing its usual "is XYZ supported" tests, the `--no-threads` option caused the checks to fail, so cmake believed that for example `LIBCXX_SUPPORTS_FNO_EXCEPTIONS_FLAG` was false. But the cmake was overall still successful. So we got an apparently successful libcxx build but it didn't have the settings that were requested. This manifested as weird issues in a faraway place later. It took a lot of time to track down.
>>
>> It's not clear to me why the `--no-threads` made the "is XYZ supported" checks fail but the actual build still succeeded. I'm pretty worried though about introducing this kind of silent behavior change. Do you think other projects may be susceptible to such a problem? Should the lack of `--no-threads` compatibility be reconsidered for the 11 release branch?
>
> Sam is the code owner of the WebAssembly backend. He makes the decision whether `--no-threads` needs to be supported.
>
> Personally I recognize that the removal of the option caused some friction but (at least for COFF/ELF) I don't think the use cases are critical enough to be supported.

I'm OK with fixing this downstream in this case I think.   There should be very few users of this flag with wasm-ld but also I wouldn't want the flag names for the same feature to be different between different lld backends.

In terms of the way this problem manifested, It makes sense that the cmake feature detection would fail because it wouldn't be able to link any of the test programs it was building.    I imagine the overall project still built because there are no executables in this case, only libraries, so there is no link step in the build, only llvm-ar.    For most projects this wouldn't be and issue and they would see a clear link failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76885