Skip to content

More wasm SIMD updates #1092

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

Merged
merged 3 commits into from
Mar 21, 2021
Merged

More wasm SIMD updates #1092

merged 3 commits into from
Mar 21, 2021

Conversation

alexcrichton
Copy link
Member

  • Sync with the latest LLVM which has a few new intrinsic names
  • Move explicit tests back to assert_instr since assert_instr now
    supports specifying const-generic arguments inline.
  • Enable tests where wasmtime implements the instruction as well as LLVM.
  • Ensure there are tests for all functions that can be tested at this
    time (those that aren't unimplemented in wasmtime).

There's still a number of assert_instr tests that are commented out.
These are either because they're unimplemented in wasmtime at the moment
or LLVM doesn't have an implementation for the instruction yet.

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

I'll note that this is blocked on rust-lang/rust#83285 for landing, but I wanted to go ahead and put this up for review if others have time.

@Amanieu
Copy link
Member

Amanieu commented Mar 20, 2021

I had a quick look through and the changes LTGM.

@alexcrichton alexcrichton force-pushed the wasm-sync branch 2 times, most recently from ed370fa to c8bfaf9 Compare March 20, 2021 18:05
@alexcrichton
Copy link
Member Author

I was struck with inspiration after reading over #1093 and I've updated this to also improve support for atomic intrinsics by always including them. Updates to the upstream proposal mean that this is now feasible (yay!) which means we are now also testing them on CI that they create the right instruction (and should catch bugs like #1093 earlier)

This is still, however, waiting on a nightly to get fully enabled. I realized that I was testing with a build of Wasmtime, however, that had a compile-time feature enabled to get more SIMD things working so I updated the docker container for wasm tests to compile a local version of Wasmtime. This does add ~5min to the wasm builder, although Wasmtime will hopefully change in the near future so we can use the precompiled binaries again.

* Sync with the latest LLVM which has a few new intrinsic names
* Move explicit tests back to `assert_instr` since `assert_instr` now
  supports specifying const-generic arguments inline.
* Enable tests where wasmtime implements the instruction as well as LLVM.
* Ensure there are tests for all functions that can be tested at this
  time (those that aren't unimplemented in wasmtime).

There's still a number of `assert_instr` tests that are commented out.
These are either because they're unimplemented in wasmtime at the moment
or LLVM doesn't have an implementation for the instruction yet.
While they're not very useful in single-threaded mode this makes them
more useful for building libraries because you don't have to always
recompile the standard library to get the desired effect. Additionally
it helps us enable tests on CI for these functions, since the
instructions will now validate without shared memory (thankfully!).
@alexcrichton
Copy link
Member Author

Ok should be good to go now!

@Amanieu
Copy link
Member

Amanieu commented Mar 21, 2021

As part of #1066 I changed the intrinsics in memory.rs to use const generics. However I left the existing check for invalid values as a runtime check.

What do you think of changing this to use static_assert! like all the other intrinsics that were converted to use const generics?

@alexcrichton
Copy link
Member Author

Oh perfect! I was going to bring that up on the stabilization issue because I agree that these should all have static assertions that only valid constants can be provided. I'll update all wasm bits to have a static assertion.

@alexcrichton
Copy link
Member Author

Ok I've added a static_assert! for all const-generics used for wasm simd functions, and I've also updated the memory intrinsics to have a static assert that the memory is 0.

@Amanieu Amanieu merged commit 13c8bc5 into rust-lang:master Mar 21, 2021
@Amanieu
Copy link
Member

Amanieu commented Mar 21, 2021

Great work!

@alexcrichton alexcrichton deleted the wasm-sync branch March 21, 2021 17:12
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