Skip to content

core: Use __jcvt() intrinsic on AArch64 for f64→i32 conversion#22138

Draft
linkmauve wants to merge 2 commits intoruffle-rs:masterfrom
linkmauve:jsconv
Draft

core: Use __jcvt() intrinsic on AArch64 for f64→i32 conversion#22138
linkmauve wants to merge 2 commits intoruffle-rs:masterfrom
linkmauve:jsconv

Conversation

@linkmauve
Copy link

In #21780, an optimisation has been added to use the fjcvtzs ARMv8.3 instruction when available, to convert a f64 into an i32. This made me wonder why core::arch::aarch64 didn’t have an intrinsic for this instruction, so I implemented it in stdarch, which got pulled in Rust yesterday (see the tracking issue).

This PR makes use of this new intrinsic to remove the unsafe asm!() block, and simplify the code.

@kjarosh
Copy link
Member

kjarosh commented Nov 4, 2025

Thanks for taking care of it! Ruffle uses stable Rust, so we have to wait until this feature is stabilized.

@linkmauve linkmauve marked this pull request as draft November 4, 2025 09:02
@linkmauve
Copy link
Author

linkmauve commented Nov 4, 2025

I’ll try to move forward with the stabilization, in the meantime should I close this PR and reopen it when the intrinsic got stabilized, or should I keep it open as a draft? I originally thought it would be better for stabilization if projects using it were actually using the intrinsic.

@kjarosh
Copy link
Member

kjarosh commented Nov 4, 2025

It's fine to leave it as a draft.

@folkertdev
Copy link

There are a bunch of red CI actions here, so just checking: the intrinsic works as expected? If so then I'd say we can start the stabilization process in ~ 2 months probably.

@kjarosh
Copy link
Member

kjarosh commented Nov 9, 2025

In order to check whether the intrinsic works as expected someone with the right hardware would have to run it locally, or GitHub actions would have to be changed (in this PR) to use nightly instead of stable. Currently they are failing because they use stable where this feature is not available.

@folkertdev
Copy link

Well yes either of those would do. I think that with that practical validation, we could move forward with stabilizing the intrinsic.

In ruffle-rs#21780, an optimisation has been added to use the fjcvtzs ARMv8.3
instruction when available, to convert a f64 into an i32.  This made me
wonder why core::arch::aarch64 didn’t have an intrinsic for this
instruction, so I implemented it in stdarch[1], which got pulled in Rust
yesterday[2] (see the tracking issue[3]).

This PR makes use of this new intrinsic to remove the unsafe asm!()
block, and simplify the code.

[1] rust-lang/stdarch#1938
[2] rust-lang/rust#148402
[3] rust-lang/rust#147555
@folkertdev
Copy link

Well, the arm and macos jobs succeeded, I guess that means it works?

@linkmauve
Copy link
Author

With only nightly enabled, the tests now pass on both ubuntu-24.04-arm and macos-15!

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