[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

kripken added a comment. In D77908#1977039 , @sbc100 wrote: > As a less controversial version of this change I could instead create a new > CPU called `current` and leave `generic` as is (basically leave it at mvp) > until we can agree that a features is

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sbc100 added a comment. I don't have documented plan anywhere. I simply observed that we have CPUs called `mvp` and `generic` and it made sense to start including more things in the generic CPU, leaving the `mvp` option open to those who want to be conservative. I don't think its a huge issue

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

kripken added a comment. Is the general plan for LLVM documented somewhere? It's not obvious to me why something being in the wasm spec means it should be enabled by default in LLVM. (It's also not obvious to me that is wrong! I'm just not sure what the reasoning is here.) In particular, somet

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sunfish added a comment. Looks good! I don't have anything to add beyond Thomas' review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77908/new/ https://reviews.llvm.org/D77908 ___ cfe-commit

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sbc100 updated this revision to Diff 256713. sbc100 added a comment. err.. arc diff did something funny. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77908/new/ https://reviews.llvm.org/D77908 Files: clang/lib/Basic/Targets/WebAssembly.cpp cl

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sbc100 updated this revision to Diff 256712. sbc100 marked 7 inline comments as done. sbc100 added a comment. feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77908/new/ https://reviews.llvm.org/D77908 Files: clang/lib/Basic/Targets/WebAss

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sbc100 added inline comments. Comment at: clang/lib/Basic/Targets/WebAssembly.h:33 - bool HasNontrappingFPToInt = false; + bool HasNontrappingFPToInt = true; bool HasSignExt = false; tlively wrote: > It seems strange to change the default here. x86 initial

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

tlively added a comment. Do we have tests that check that passing `-nontrapping-fptoint` to llc (or clang) successfully disables the feature? Comment at: clang/lib/Basic/Targets/WebAssembly.h:33 - bool HasNontrappingFPToInt = false; + bool HasNontrappingFPToInt = true;

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

sbc100 created this revision. Herald added subscribers: cfe-commits, jfb, arphaman, sunfish, aheejin, hiraditya, jgravelle-google, dschuff. Herald added a project: clang. Since this feature is now merged into the upstream wasm spec it makes sense to enable it by default, at least for the `default