Skip to content

[wasm2js] Support nontrapping float-to-int conversions directly in JS - #9019

Open
sbc100 wants to merge 1 commit into
mainfrom
wasm2js-nontrapping-fptoint
Open

[wasm2js] Support nontrapping float-to-int conversions directly in JS#9019
sbc100 wants to merge 1 commit into
mainfrom
wasm2js-nontrapping-fptoint

Conversation

@sbc100

@sbc100 sbc100 commented Aug 19, 2026

Copy link
Copy Markdown
Member

Now that 64-bit trunc_sat instructions are supported directly in I64ToI32Lowering (#9017), and 32-bit trunc_sat instructions are handled directly in wasm2js (~~expr and ~~expr >>> 0), we no longer need to run llvm-nontrapping-fptoint-lowering when --emscripten is passed.

In wasm2js, trapping float-to-int operations (i32.trunc_* and i64.trunc_*) are already lowered to non-trapping JS conversions (~~expr and float arithmetic in I64ToI32Lowering), which do not trap on out-of-range values or NaN/Infinity (matching C/C++ UB semantics rather than strict Wasm trapping semantics).

Removing llvm-nontrapping-fptoint-lowering extends that same direct approach to trunc_sat instructions under --emscripten, eliminating branching overhead and reducing code size.

Finally, add a commit to LLVMNontrappingFPToIntLowering.cpp about it have no known users after this change.

@sbc100
sbc100 requested a review from a team as a code owner August 19, 2026 21:30
@sbc100
sbc100 requested review from aheejin and removed request for a team August 19, 2026 21:30
@sbc100
sbc100 requested a review from dschuff August 19, 2026 21:32
@sbc100

sbc100 commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

I ran the whole wasm2js0, wasm2js1 and wasm2js2 test suites in emscripten with this change and (to my surprise) all tests passed.

@sbc100
sbc100 requested a review from kripken August 19, 2026 21:33
@sbc100
sbc100 force-pushed the wasm2js-nontrapping-fptoint branch from d3f41bf to 83f7451 Compare August 19, 2026 21:36
@sbc100
sbc100 requested review from tlively and removed request for aheejin August 19, 2026 21:37
Comment thread src/passes/LLVMNontrappingFPToIntLowering.cpp
@sbc100
sbc100 force-pushed the wasm2js-nontrapping-fptoint branch from 83f7451 to 7b14553 Compare August 19, 2026 22:33
@sbc100

sbc100 commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

@kripken WDYT about these two stacked PRs combined? Do you think this approach is OK? I can't say I fully understand all the edge cases but the emscripten tests suite seems to pass fully for wasm2js, which is the only user I know of these passes.

@kripken

kripken commented Aug 20, 2026

Copy link
Copy Markdown
Member

I think this makes sense to me. I don't see a problem, at least.

To get more confidence, perhaps run the binaryen fuzzer locally, specifically the wasm2js one there.

@sbc100

sbc100 commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

I think this makes sense to me. I don't see a problem, at least.

To get more confidence, perhaps run the binaryen fuzzer locally, specifically the wasm2js one there.

I'm not sure I've ever done that before. looking it up now.. What would the failure mode be that I am looking for?

@sbc100

sbc100 commented Aug 20, 2026

Copy link
Copy Markdown
Member Author

Looks like we might need to enable these features in the wasm2js fuzzer first: #9024

@sbc100
sbc100 force-pushed the wasm2js-nontrapping-fptoint branch from 7b14553 to 149eaef Compare August 20, 2026 15:36
Base automatically changed from i64-lowering-trunc-sat to main August 20, 2026 18:51
@sbc100
sbc100 force-pushed the wasm2js-nontrapping-fptoint branch from 149eaef to 5b2a3f9 Compare August 20, 2026 18:51
Now that 64-bit `trunc_sat` instructions are supported directly in
`I64ToI32Lowering` (#9017), and 32-bit `trunc_sat` instructions are
handled directly in `wasm2js` (`~~expr` and `~~expr >>> 0`), we no
longer need to run `llvm-nontrapping-fptoint-lowering` when
`--emscripten` is passed.

In `wasm2js`, trapping float-to-int operations (`i32.trunc_*` and
`i64.trunc_*`) are already lowered to non-trapping JS conversions
(`~~expr` and float arithmetic in `I64ToI32Lowering`), which do not
trap on out-of-range values or NaN/Infinity (matching C/C++ UB semantics
rather than strict Wasm trapping semantics).

Removing `llvm-nontrapping-fptoint-lowering` extends that same direct
approach to `trunc_sat` instructions under `--emscripten`, eliminating
branching overhead and reducing code size.
@sbc100
sbc100 force-pushed the wasm2js-nontrapping-fptoint branch from 5b2a3f9 to f0a2110 Compare August 20, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants