-
Notifications
You must be signed in to change notification settings - Fork 292
Add remaining integer AVX 512f comparison intrinsics #864
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
Conversation
r? @gnzlbg (rust_highfive has picked a reviewer for you, use r? to override) |
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.
This should be ready for review.
#[target_feature(enable = "avx512f")] | ||
#[rustc_args_required_const(3)] | ||
#[cfg_attr(test, assert_instr(vpcmp, op = 0))] | ||
pub unsafe fn _mm512_mask_cmp_epu64_mask( |
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.
Arguably, all of the other intrinsics should be implemented on top of this intrinsic. My preference would be to declare victory and move on, but I can make the change if you would like.
#[target_feature(enable = "avx512f")] | ||
#[rustc_args_required_const(2)] | ||
#[cfg_attr(test, assert_instr(vpcmp, op = 0))] | ||
pub unsafe fn _mm512_cmp_epu64_mask(a: __m512i, b: __m512i, op: _MM_CMPINT_ENUM) -> __mmask8 { |
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.
It's kind of annoying to have the full implementation duplicated here. When I attempted to call _mm512_mask_cmp_epu64_mask
here, the constness of op
was not propagated along, so I got compiler errors that the argument needed to be a constant.
Are you planning on adding the epi32/epu32 intrinsics as well. Those are also part of AVX512F. |
I'm a little confused by the documentation for the 32 bit comparisons. Most of the 32 bit comparisons read The I have been reading that documentation as being an implicit |
I believe the condition is AVX512F OR KNC. I would ignore KNC for now and just gate it on AVX512F only. |
I've added the 32 bit integer comparisons and updated |
Closing since #866 also contains these changes. |
This is just a draft. I created the PR in order to get CI to run.