Skip to content

WIP: Update SIMD instructions #105

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
Oct 1, 2020

Conversation

silvanshade
Copy link
Contributor

This PR updates SIMD support to reflect recent syntax changes (and new instructions) in the proposal repo.

This PR allows wast to successfully parse all of the test files in the upstream WebAssembly/simd repo (tested with the testsuite for the webassembly language server, which uses wast) but unfortunately it breaks a lot of the tests in tests/local, tests/testsuite, and tests/wabt which still use the old syntax.

I tried fixing some of the tests in tests/local but wasn't sure about some of the errors. I also tried regenerating the upstream WebAssembly/testsuite but the update-testsuite.sh script fails on several of the repos with unresolvable merge conflicts and I wasn't sure how to work around that without messing up the script workflow.

So for the meantime I will just leave this here.

@alexcrichton
Copy link
Member

Thanks for this! Would you be ok linking some of the PRs on the simd repo here for posterity too where the renames happened?

Additionally I think this is fine to land and not block on webassembly/testsuite and/or webassembly/wabt being updated, there's various locations throughout roundtrip.rs where test are ignored and you can add some TODO/FIXME items to get removed in the future.

@silvanshade
Copy link
Contributor Author

silvanshade commented Sep 25, 2020

Here are links to the respective PRs where the recent renaming and additions to the SIMD proposal were made:

WebAssembly/simd#321
WebAssembly/simd#322
WebAssembly/simd#341
WebAssembly/simd#344

I've also created PRs for WebAssembly/testsuite and WebAssembly/wabt:

WebAssembly/testsuite#32
WebAssembly/wabt#1553

@silvanshade
Copy link
Contributor Author

I've updated the PR, temporarily pointing my local branches of testsuite and wabt until the upstream PRs are merged. All of the tests should pass now.

Note that I had to modify Integer parsing in 98c83b1. This was needed in order to be able to fail (correctly) when parsing lane indices with sign prefixes. See here.

@silvanshade silvanshade marked this pull request as ready for review September 27, 2020 09:53
@alexcrichton
Copy link
Member

Looks great to me, thanks again for all your help here on this!

To bikeshed a bit, though, the name "signededness" is a bit overloaded with integers themselves and the token in front, and I was a bit perplexed at Signededness::Unsigned meaning "no sign token" when I first saw it. That and I always have a hard time spelling and typing "signededness" (even now I'm not sure that it's right...)

Perhaps a name like SignToken::{Plus, Minus, None}? Or Option<SignToken::{Plus, Minus}>?

@silvanshade silvanshade force-pushed the update-simd branch 3 times, most recently from 8830ce4 to 0abaca0 Compare September 27, 2020 22:32
@alexcrichton
Copy link
Member

I've got a commit at alexcrichton@954df9d which updates to the upstream testsuite module now with some other assorted fixes that cropped up, feel free to cherry-pick that here or take what you like from it! With the submodules set back to upstream repos I think this should be good to merge.

@silvanshade silvanshade force-pushed the update-simd branch 3 times, most recently from 7cdd02d to 8b39a4d Compare October 1, 2020 03:06
* Update the upstream testsuite to the current master branch
* Update wabt to the current master branch
* Get the test suite working with SIMD/memory64 changes (mostly just
  ignoring memory64 and simd with wabt)
@silvanshade
Copy link
Contributor Author

@alexcrichton I think this should be good to go now.

@alexcrichton alexcrichton merged commit 8ec7af4 into bytecodealliance:main Oct 1, 2020
@alexcrichton
Copy link
Member

Perfect, thanks again!

dhil pushed a commit to dhil/wasm-tools that referenced this pull request Jun 21, 2024
This is an automated pull request from CI to release
wasmfx-tools 1.211.2 when merged. The commit
message for this PR has a marker that is detected by CI to create
tags and publish crate artifacts.

When first opened this PR will not have CI run because it is generated
by a bot. A maintainer should close this PR and then reopen it to
trigger CI to execute which will then enable merging this PR.

Co-authored-by: Auto Release Process <[email protected]>
dhil added a commit to dhil/wasm-tools that referenced this pull request Jun 21, 2024
dhil added a commit to dhil/wasm-tools that referenced this pull request Jun 21, 2024
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