majaha wrote: > These are just non-wasm options, so they mean nothing to wasm.
Yes, and using them gives no warning. > Wasm simply doesn't do anything with respect to these options and wasm is not > overriding them. --trap-unreachable is absolutely overridden, in exactly the same place as --no-trap-after-noreturn. It happens on the previous line of code. > they wouldn't know that their request was silently rejected/overridden. > To say it another way, the difference is, Wasm doesn't understand > --xcoff-traceback-table=true and will do nothing with respect to it. But Wasm > understands --no-trap-after-noreturn and will silently reject it after this > PR. There's no difference between "doing nothing with respect to it" and "silently rejecting", it's the same thing. The wasm backend understands neither --xcoff-traceback-table, --trap-unreachable, *or* --no-trap-after-noreturn (the fact that --no-trap-after-noreturn affects the wasm output despite being conceptually unsupported *is* the bug). None of those are supported by the wasm backend, and they are all equally silently ignored. > I have been kind of confused by your characterization of this as "bugfix", > given that this does not change anything functionality-wise. > ...So I'm not sure if I agree that this PR is a bugfix. I consider this as an > improvement adding a failsafe mechanism. It *does* change things functionality wise. [Look at the tests](https://github.com/llvm/llvm-project/pull/65876/commits/7e834694b09e6a7787674879442b5f0d0c92eb22#diff-1b973bc4d02665dc4a2d31916df466a5bf53c64ef70aa701ef579fc67dfbd2e4L96), the output has changed with this pull request from invalid to valid. If that's not a bug fix, I don't know what is. > What's your distinction criteria for "real contradiction" and "hints or > suggestions"? A real contradiction would be asking for two incompatible target options, like [-exception-model=wasm -enable-emscripten-cxx-exceptions](https://godbolt.org/z/n77WeqaT6). TrapUnreachable is a much more wishy-washy request, it's frequently ignored across the codebase, and [using it doesn't even guarantee that unreachable will produce a trap](https://godbolt.org/z/evdrEvh1h). It's just a code-hardening hint that reduces the effect of runaway code in some instances. To put it plainly: the --trap-unreachable and --no-trap-after-noreturn options *are not supported* on the wasm backend, and they are silently ignored just like every other unsupported option. This is consistent with the behaviour both [in the wasm backend](https://github.com/llvm/llvm-project/blob/21f8bc25adba762ab06e26a7dd50da78fcd17528/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp#L129), and across the codebase: [1](https://github.com/llvm/llvm-project/blob/aa8f5e6156821aec1fed595f2e13d69655ec3311/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp#L340), [2](https://github.com/llvm/llvm-project/blob/aa8f5e6156821aec1fed595f2e13d69655ec3311/llvm/lib/Target/ARM/ARMTargetMachine.cpp#L254), [3](https://github.com/llvm/llvm-project/blob/aa8f5e6156821aec1fed595f2e13d69655ec3311/llvm/lib/Target/X86/X86TargetMachine.cpp#L237). https://github.com/llvm/llvm-project/pull/65876 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
