[i64-to-i32-lowering] Lower nontrapping float-to-int conversions - #9017
Conversation
I guess we'll have to lower the wide arithmetic instructions next!
Looks like this got a little mangled, so I'm not sure what it means. |
| case TruncSatSFloat32ToInt64: | ||
| case TruncUFloat32ToInt64: | ||
| case TruncSatUFloat32ToInt64: { |
There was a problem hiding this comment.
Surely there are differences in semantics that we would have to handle here?
There was a problem hiding this comment.
I would have thought so too.. maybe the existing implementation (outlined above) already happens to do saturation rather than trapping?
There was a problem hiding this comment.
OK some more details here, aided by AI:
The existing implementation in I64ToI32Lowering.cpp was already compatible with saturating conversions for several key reasons:
1. wasm2js Semantics Already Do Not Trap
I64ToI32Lowering
is primarily designed for the wasm2js pipeline. In wasm2js, standard
trapping truncation (i32.trunc_* / i64.trunc_*) is lowered to JavaScript
bitwise and floating-point operations (e.g., ~~expr / (~~expr) >>> 0 in
wasm2js.h),
which never trap on out-of-range values, infinities, or NaN. Because trapping
semantics were already not strictly emulated, standard trunc and saturating
trunc_sat converge on the same JS-level lowering.
2. The Arithmetic Naturally Handles Saturating Edge Cases
3. Signed and Unsigned Already Shared the Same Logic
Even for non-saturating conversions (TruncSFloat*ToInt64 and
TruncUFloat*ToInt64),
lowerTruncFloatToInt
already routed both signed and unsigned opcodes to the exact same logic
because the two's complement bit decomposition works identically for positive
and negative values. The saturating variants (TruncSatS* and TruncSatU*)
fit directly into the same pattern.
4. Consistency with 32-bit trunc_sat Lowering
In wasm2js.h,
32-bit saturating operations (TruncSatSFloat32ToInt32,
TruncSatUFloat32ToInt32, etc.) already fall through to the exact same
codegen as the trapping 32-bit conversions (~~expr and (~~expr) >>> 0).
Reusing lowerTruncFloatToInt
for 64-bit trunc_sat maintains parity between 32-bit and 64-bit lowering.
There was a problem hiding this comment.
(raced with the previous comment)
It's basically impossible to tell because there are no comments 🥲 I see the output uses a trapping 32-bit truncation, so it seems possible that it traps.
Do you know how this is tested beyond that single lit test? Are we running the spec tests through wasm2js, perhaps?
There was a problem hiding this comment.
Ok, I buy that wasm2js doesn't care about preserving these traps. but is there precedent in i64-to-i32-lowering for changing trap behavior?
If not, maybe this is still ok, but we would want to document that at the top of the file, at least.
There was a problem hiding this comment.
I believe the existing conversions are the ones that are non-confirming in that they do not trap when they should.
These new ones I'm adding are more conforming than the existing ones because they do not expect trapping behavior, right? They expect the saturating behaviour.
So, yes, I believe there is precedent in i64-to-i32-lowering for changing trap behavior.. but that is not relevant to this change IIUC.
Fixed. |
|
Perhaps the purpose here will be more explicit if you also consider the followup change in this PR stack #9019 |
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.
|
OK, lets add some comments before we land this change: #9022 |
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
Ok, LGTM with that documentation.
… (NFC) Document that `I64ToI32Lowering` is designed for the `wasm2js` pipeline and does not preserve strict WebAssembly trapping semantics for float-to-int conversions. Also add comments in `lowerTruncFloatToInt` noting that the emitted 32-bit truncations are translated to JavaScript bitwise operations in `wasm2js` and thus do not trap on out-of-range values, NaN, or infinity. See: #9017
9fe6a97 to
5a229b3
Compare
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.
#9022) Document that `I64ToI32Lowering` is designed for the `wasm2js` pipeline and does not preserve strict WebAssembly trapping semantics for float-to-int conversions. Also add comments in `lowerTruncFloatToInt` noting that the emitted 32-bit truncations are translated to JavaScript bitwise operations in `wasm2js` and thus do not trap on out-of-range values, NaN, or infinity. See: #9017
5a229b3 to
07109c3
Compare
Add support for `TruncSatSFloat32ToInt64`, `TruncSatUFloat32ToInt64`, `TruncSatSFloat64ToInt64`, and `TruncSatUFloat64ToInt64` to `I64ToI32Lowering.cpp`. In Emscripten mode (`wasm2js --emscripten`), this is effectively a no-op because `llvm-nontrapping-fptoint-lowering` runs earlier in the pipeline and rewrites all `TruncSat` operations into trapping conversions before `i64-to-i32-lowering` executes. However, landing this is still useful because: 1. Standalone `wasm2js` (without `--emscripten`) does not run `llvm-nontrapping-fptoint-lowering`, so modules with `i64.trunc_sat_*` instructions would otherwise fail during JS generation. 2. Direct invocations of `wasm-opt --flatten --i64-to-i32-lowering` will now correctly eliminate all 64-bit operations on modules using the nontrapping float-to-int feature. 3. It ensures `I64ToI32Lowering` is self-contained and handles all wasm i64-producing float truncation opcodes without relying on preceding lowering passes. 4. Since we removed the use of the `nontrapping-fptoint-lowering` pass in emscripten itself, we I'm hoping we can followup be remove it use here and marking it for removal.
07109c3 to
df127c5
Compare
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.
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.
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.
2 participants
Add support for
TruncSatSFloat32ToInt64,TruncSatUFloat32ToInt64,TruncSatSFloat64ToInt64, andTruncSatUFloat64ToInt64toI64ToI32Lowering.cpp.In Emscripten mode (
wasm2js --emscripten), this is effectively a no-op becausellvm-nontrapping-fptoint-loweringruns earlier in the pipeline and rewrites allTruncSatoperations into trapping conversions beforei64-to-i32-loweringexecutes.However, landing this is still useful because:
wasm2js(without--emscripten) does not runllvm-nontrapping-fptoint-lowering, so modules withi64.trunc_sat_*instructions would otherwise fail during JS generation.wasm-opt --flatten --i64-to-i32-loweringwill now correctly eliminate all 64-bit operations on modules using the nontrapping float-to-int feature.I64ToI32Loweringis self-contained and handles all wasm i64-producing float truncation opcodes without relying on preceding lowering passes.nontrapping-fptoint-loweringpass in emscripten itself, I'm hoping we can followup by removing its use here too and marking it for removal.