-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Add intrinsic to create an integer bitmask from a vector mask #57269
Conversation
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Seems reasonable to me! |
I forgot to mention it in the PR description, but the reason we need an intrinsics for this is that we have to truncate the vector lanes to an I've amended the PR message with this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM in principle, but I'd prefer to not support float vectors. Also some nits about the implementation and tests.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said I'd still prefer to drop support for float elements, plus a style nit. You can also squash the commits if you don't mind.
I've squashed the commits and hopefully fixed the last issues. |
@rkruppe this is awaiting your review :) |
LGTM, sorry for the delay! @bors r+ |
📌 Commit 322a51e216112192d94db949b0fb63be1447cdb4 has been approved by |
⌛ Testing commit 322a51e216112192d94db949b0fb63be1447cdb4 with merge 9ab79f87654feb32c90ba29073ccdbfbe0572f61... |
💔 Test failed - checks-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
So instead of hardcoding the LLVM identifiers (e.g. |
That should do it, but please use variables to test that the values are actually used as expected. |
@rkruppe I've done that, cool, I didn't knew about this. That document is really helpful. |
r=me when travis passes |
@bors r+ |
📌 Commit 785f529 has been approved by |
Add intrinsic to create an integer bitmask from a vector mask This PR adds a new simd intrinsic: `simd_bitmask(vector) -> unsigned integer` that creates an integer bitmask from a vector mask by extracting one bit of each vector lane. This is required to implement: rust-lang/packed_simd#166 . EDIT: the reason we need an intrinsics for this is that we have to truncate the vector lanes to an `<i1 x N>` vector, and then bitcast that to an `iN` integer (while making sure that we only materialize `i8`, ... , `i64` - that is, no `i1`, `i2`, `i4`, types), and we can't do any of that in a Rust library. r? @rkruppe
☀️ Test successful - checks-travis, status-appveyor |
☀️ Test successful - checks-travis, status-appveyor |
This PR adds a new simd intrinsic:
simd_bitmask(vector) -> unsigned integer
that creates an integer bitmask from a vector mask by extracting one bit of each vector lane.This is required to implement: rust-lang/packed_simd#166 .
EDIT: the reason we need an intrinsics for this is that we have to truncate the vector lanes to an
<i1 x N>
vector, and then bitcast that to aniN
integer (while making sure that we only materializei8
, ... ,i64
- that is, noi1
,i2
,i4
, types), and we can't do any of that in a Rust library.r? @rkruppe