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.

Sign up for GitHub

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

Jump to bottom

Improve wasm32 to_array performance #51

Closed
workingjubilee opened this issue Dec 8, 2020 · 5 comments · Fixed by #142
Closed

Improve wasm32 to_array performance #51

workingjubilee opened this issue Dec 8, 2020 · 5 comments · Fixed by #142
Labels
O-Wasm WebAssembly's virtual architecture

Comments

@workingjubilee
Copy link
Member

It's not clear why these are happening, they seem to be fixed in recent PRs to the mask API but not on purpose? This is being opened to make sure they get addressed eventually anyways.
@workingjubilee workingjubilee added the O-Wasm WebAssembly's virtual architecture label Dec 8, 2020
@miguelraz miguelraz mentioned this issue Jan 27, 2021
Merged
@workingjubilee
Copy link
Member Author

These apparently are mysteriously OK now. ¯\_(ツ)_/¯

@workingjubilee
Copy link
Member Author

workingjubilee commented Jan 27, 2021 •
edited
Loading

Oh, it's because of this bodge. Let's fix this... someday. https://github.com/rust-lang/stdsimd/blob/b931c15c0b9fad90b8020ea5a5ad54eef7519edf/crates/core_simd/src/macros.rs#L54-L73

@workingjubilee workingjubilee changed the title Address mask128x4 on wasm SIMD failures Improve wasm32 to_array performance Jan 27, 2021
@workingjubilee
Copy link
Member Author

rust-lang/rust#80108 remains an issue on the latest LLVM. I do not know if this is an issue with rustc or with LLVM.

@calebzulawski
Copy link
Member

Fairly confident it's an LLVM bug, since that's a completely valid bitcast. I never actually reduced it or submitted it to LLVM though. I should do that...

@workingjubilee
Copy link
Member Author

My rustc can now compile the trivial example given in #80108, so I believe we can take another crack at removing this bodge, but we should rebuild our wasm test suite to guarantee it's actually correct.

@calebzulawski calebzulawski mentioned this issue Jun 28, 2021
Merged
workingjubilee added a commit that referenced this issue Jul 24, 2021
@workingjubilee
Merge pull request #142 from rust-lang/feature/traits
82e3405 
Combine LanesAtMost32 and SimdArray into a single trait "Vector"

Attempts to fix some unresolved questions in #139 regarding `SimdArray` having a generic parameter.

In particular, this made it not appropriate for replacing `LanesAtMost32`.  Additionally, it made it impossible to use in a context where you otherwise don't know the lane count, e.g. `impl Vector`.

An unfortunate side effect of this change is that scatter/gather no longer work in the trait (nor does anything else that references the lane count in a type.  This requires the super-unstable `const_evaluatable_checked` feature).

I also threw in the change from `as_slice` to `as_array` as discussed in zulip, and fixes #51.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
O-Wasm WebAssembly's virtual architecture
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Combine LanesAtMost32 and SimdArray into a single trait "Vector" rust-lang/portable-simd
2 participants
@calebzulawski @workingjubilee