Skip to content

[i64-to-i32-lowering] Lower nontrapping float-to-int conversions #9017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sbc100 merged 1 commit into from
Aug 20, 2026
+416 −4
Merged

[i64-to-i32-lowering] Lower nontrapping float-to-int conversions #9017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/passes/I64ToI32Lowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,9 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
BinaryOp ge, gt, min, div, sub;
switch (curr->op) {
case TruncSFloat32ToInt64:
case TruncUFloat32ToInt64: {
case TruncSatSFloat32ToInt64:
case TruncUFloat32ToInt64:
case TruncSatUFloat32ToInt64: {
Comment on lines +702 to +704

@tlively tlively Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely there are differences in semantics that we would have to handle here?

@sbc100 sbc100 Aug 19, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have thought so too.. maybe the existing implementation (outlined above) already happens to do saturation rather than trapping?

@sbc100 sbc100 Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tlively tlively Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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?

@tlively tlively Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sbc100 sbc100 Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

litZero = Literal((float)0);
litOne = Literal((float)1);
u32Max = Literal(((float)UINT_MAX) + 1);
Expand All @@ -717,7 +719,9 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
break;
}
case TruncSFloat64ToInt64:
case TruncUFloat64ToInt64: {
case TruncSatSFloat64ToInt64:
case TruncUFloat64ToInt64:
case TruncSatUFloat64ToInt64: {
litZero = Literal((double)0);
litOne = Literal((double)1);
u32Max = Literal(((double)UINT_MAX) + 1);
Expand Down Expand Up @@ -930,9 +934,13 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
case ExtendUInt32:
case WrapInt64:
case TruncSFloat32ToInt64:
case TruncSatSFloat32ToInt64:
case TruncUFloat32ToInt64:
case TruncSatUFloat32ToInt64:
case TruncSFloat64ToInt64:
case TruncSatSFloat64ToInt64:
case TruncUFloat64ToInt64:
case TruncSatUFloat64ToInt64:
case ReinterpretFloat64:
case ConvertSInt64ToFloat32:
case ConvertSInt64ToFloat64:
Expand Down Expand Up @@ -981,9 +989,13 @@ struct I64ToI32Lowering : public WalkerPass<PostWalker<I64ToI32Lowering>> {
lowerReinterpretInt64(curr);
break;
case TruncSFloat32ToInt64:
case TruncSatSFloat32ToInt64:
case TruncUFloat32ToInt64:
case TruncSatUFloat32ToInt64:
case TruncSFloat64ToInt64:
case TruncSatSFloat64ToInt64:
case TruncUFloat64ToInt64:
case TruncSatUFloat64ToInt64:
lowerTruncFloatToInt(curr);
break;
case ConvertSInt64ToFloat32:
Expand Down
Loading
Loading