Skip to content
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.

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

Use WebAssembly SIMD instructions #34

Merged
merged 3 commits into from
Jul 29, 2021

Conversation

CryZe
Copy link
Contributor

@CryZe CryZe commented May 25, 2021

WebAssembly SIMD is almost here. Chrome 91 releases today. It is the first browser to ship WASM SIMD. Firefox 89 will follow in a bit over a week as well. Rust also intends to stabilize these instructions very soon. Until then this PR will stay WIP.

@CryZe
Copy link
Contributor Author

CryZe commented May 25, 2021

I'm also preparing a PR with NEON intrinsics 😄 (which they also intend to stabilize soon, probably a bit later than the WASM ones though)

@RazrFalcon
Copy link
Collaborator

  1. I would suggest adding this to safe_arch first. This crate is intended to be unsafe-free.
  2. How can we test this on CI?

@CryZe
Copy link
Contributor Author

CryZe commented May 25, 2021

I don't think safe-arch is necessary here, at least when it comes to the unsafety concern. They are soon, before stabilizing, making those intrinsics safe.

For testing we could probably run the tests with wasmtime.

@RazrFalcon
Copy link
Collaborator

RazrFalcon commented May 25, 2021

I want this crate to be compilable with #![forbid(unsafe)], so no unsafe blocks are allowed.

@CryZe
Copy link
Contributor Author

CryZe commented May 25, 2021

Yeah once the intrinsics are safe this will not need any unsafe anymore. They can make them safe on WASM because the runtime validates the WASM file before running it, so only instructions it supports are allowed. So either the module runs and there is no unsafety, or the module doesn't run and there's no unsafety either, because no code is ever executed.

@CryZe
Copy link
Contributor Author

CryZe commented May 25, 2021

Here is the PR to Rust: rust-lang/rust#84988

@RazrFalcon
Copy link
Collaborator

Got it. Thanks!

They are stabilized in Rust 1.54.
@CryZe CryZe force-pushed the wasm-simd-is-ready branch from 639940a to b1d1ad3 Compare June 12, 2021 01:57
@CryZe CryZe marked this pull request as ready for review June 12, 2021 01:57
@CryZe
Copy link
Contributor Author

CryZe commented Jun 12, 2021

Alright many things happened:

  1. There are now uNxM variants for all intrinsics that don't care about signedness. Earlier the u32x8_t was a "painful" mix of needing to use the signed version of shift left, but the unsigned version of shift right. Now it's all much cleaner.
  2. All the intrinsics are now entirely safe. There's no unsafe blocks left anymore.
  3. And most importantly, the intrinsics are stabilized now. (And with that I mean, it's stable in nightly, riding the train to 1.54)

I also benchmarked it with one of the examples and it's about twice as fast. It's 4:00am right now, I'll try to look into testing on CI when I wake up. (Though I believe last I tried wasmtime with SIMD it segfaulted, so yeah idk if they are ready yet)

@CryZe CryZe force-pushed the wasm-simd-is-ready branch 2 times, most recently from e8df804 to 4dcf890 Compare June 12, 2021 11:27
@CryZe
Copy link
Contributor Author

CryZe commented Jun 12, 2021

Alright CI is implemented now too. (It uses nightly for now, your decision to wait for 1.54 to release or not)

@RazrFalcon
Copy link
Collaborator

Nice! Do I understand correctly that simd128 is nightly only and will not break stable x86 builds, only wasm one. Or will it compile on stable wasm target too?

@CryZe
Copy link
Contributor Author

CryZe commented Jun 12, 2021

Yes and no. It still compiles on stable if you don't activate the feature, but LLVM features are never gated by Rust's stability, so while technically SIMD128 isn't considered stable on stable Rust, you can still activate the LLVM feature and it would fail to compile. (Which honestly is maybe worth raising an issue about)

@RazrFalcon
Copy link
Collaborator

I think we should wait until simd128 will hit stable. Mainly in case they would break something.

Anyway, thanks for you contribution.

I'm also looking forward for NEON support, because lately I got mac M1 and I actually have a hardware for testing. Sadly, NEON is still unstable in Rust for some reason. Maybe you know when they plan to make it stable?

@jrmuizel
Copy link

I asked about NEON stabilization here: rust-lang/stdarch#1125

@RazrFalcon
Copy link
Collaborator

@jrmuizel Thanks. From what I understood the core team simply doesn't have time to stabilize them.

@CryZe
Copy link
Contributor Author

CryZe commented Jun 12, 2021

As far as I understand it, they want more automatic testing first, and there's this fairly active PR that does exactly that: rust-lang/stdarch#1170

@CryZe CryZe force-pushed the wasm-simd-is-ready branch from 4dcf890 to 26afb11 Compare July 29, 2021 14:33
@RazrFalcon
Copy link
Collaborator

We can merge now?

@CryZe
Copy link
Contributor Author

CryZe commented Jul 29, 2021

Yes, 1.54 released, CI is green on stable. However should I maybe still keep nightly CI for the WASM part as well? So stable + nightly. For LLVM 13 and co. that might be coming up.

@RazrFalcon
Copy link
Collaborator

I'm fine as long as it works on stable. Nightly is not a priority.

One last question: what is the current MSRV? Can I build non-WASM variant on Rust < 1.54?

@CryZe
Copy link
Contributor Author

CryZe commented Jul 29, 2021

non-WASM: 1.45.0
WASM but no SIMD: 1.45.0
WASM but with SIMD: 1.54.0

Maybe another CI run that checks non-SIMD WASM still works on 1.45.0 though? Anyways, if not, feel free to merge.

@RazrFalcon RazrFalcon merged commit b3ba640 into linebender:master Jul 29, 2021
@RazrFalcon
Copy link
Collaborator

Done. Thanks again! I hope NEON would become stable soon, because tiny-skia performance on M1 mac is pathetic.

@CryZe CryZe deleted the wasm-simd-is-ready branch July 29, 2021 15:13
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