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

Remove u8 slice output of simd_bitmask #105990

Closed
wants to merge 1 commit into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Dec 21, 2022

related: rust-lang/miri#2734

this seems to be unused and trivially replaced by https://doc.rust-lang.org/std/primitive.i64.html#method.to_be_bytes or similar

r? @Amanieu @matthiaskrgr

@rustbot
Copy link
Collaborator

rustbot commented Dec 21, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 21, 2022
@bjorn3
Copy link
Member

bjorn3 commented Dec 21, 2022

It is not entirely useless. It is necessary for vectors with more than 128 lanes as we don't have integer types with more than 128bits.

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 21, 2022

It is not entirely useless. It is necessary for vectors with more than 128 lanes as we don't have integer types with more than 128bits.

O_o that's a thing?

Yea... I guess LLVM can actually generate these types.

Does such a vector exist right now?

@bjorn3
Copy link
Member

bjorn3 commented Dec 21, 2022

You can define it yourself using #[repr(simd)] if you do #![feature(repr_simd)]. std::arch doesn't expose any such type and std::simd::SupportedLaneCount only goes to 64 right now. Maybe once we get AVX2048? :) In any case @rust-lang/project-portable-simd will SupportedLaneCount remain no more than 128 forever?

@calebzulawski
Copy link
Member

This is used by bits of std::simd that haven't made it upstream yet (blocked on some const generics features): https://github.com/rust-lang/portable-simd/blob/master/crates/core_simd/src/masks/full_masks.rs#L162. There are already instruction sets with more than 128 lanes (Arm SVE, RISC-V V extension).

@programmerjake
Copy link
Member

You can define it yourself using #[repr(simd)] if you do #![feature(repr_simd)]. std::arch doesn't expose any such type and std::simd::SupportedLaneCount only goes to 64 right now. Maybe once we get AVX2048? :) In any case @rust-lang/project-portable-simd will SupportedLaneCount remain no more than 128 forever?

We definitely intend to support more than 128-lane Simd types in the future, so just using u128 for bitmasks is insufficient. RISC-V V and ARM SVE and SX Aurora already support more than 128 lanes.

@programmerjake
Copy link
Member

programmerjake commented Dec 21, 2022

that said, when we finally get generic-sized integer types (uint<N> for const N: usize rust-lang/rfcs#2581) that go to >128 bits, i prefer to replace all array-of-bytes code with generic-sized integers, since they're nicer to use and basically what LLVM uses internally anyway. Or even better we could use generic integers to get first-class bit vector types: Simd<int<1>, N>

@oli-obk oli-obk closed this Dec 21, 2022
@oli-obk oli-obk deleted the simd_sadness branch December 21, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants