search

RazrFalcon / tiny-skia

A tiny Skia subset ported to Rust
BSD 3-Clause "New" or "Revised" License
1.05k stars 67 forks source link

Use WebAssembly SIMD instructions #34

Closed CryZe closed 2 years ago

CryZe commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago
  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 commented 3 years ago

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 commented 3 years ago

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

CryZe commented 3 years ago

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 commented 3 years ago

Here is the PR to Rust: https://github.com/rust-lang/rust/pull/84988

RazrFalcon commented 3 years ago

Got it. Thanks!

CryZe commented 3 years ago

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 commented 3 years ago

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

RazrFalcon commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

I asked about NEON stabilization here: https://github.com/rust-lang/stdarch/issues/1125

RazrFalcon commented 3 years ago

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

CryZe commented 3 years ago

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

RazrFalcon commented 2 years ago

We can merge now?

CryZe commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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