-
Notifications
You must be signed in to change notification settings - Fork 306
Make X86 intrinsics const
#1925
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
b73089f to
f8fe8b8
Compare
42d6966 to
220fc5a
Compare
2830224 to
ec6eef8
Compare
4d1ed15 to
58c2424
Compare
1c30344 to
5459458
Compare
only ordered intrinsics have implementation in rustc-const-eval
| #[track_caller] | ||
| #[target_feature(enable = "sse2")] | ||
| pub unsafe fn assert_eq_m128i(a: __m128i, b: __m128i) { | ||
| #[rustc_const_unstable(feature = "stdarch_const_helpers", issue = "none")] |
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.
Is this actually needed? These functions aren't exported and shouldn't need stability attributes.
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.
Yes, unfortunately const-stability attributes are needed even for private functions
| #[target_feature(enable = "sse2")] | ||
| pub unsafe fn assert_eq_m128d(a: __m128d, b: __m128d) { | ||
| ct: unsafe { | ||
| assert_eq!(transmute::<_, [f64; 2]>(a), transmute::<_, [f64; 2]>(b)) |
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.
Since this is only used in tests, could we always us the ct version here? That would simplify the code.
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.
The ct version doesn't print the left and right arguments, because debug-printing is not const yet. I wanted to print the values in rt at least, so had 2 separate impls.
Also one more problem is the ct version uses simple float equality, which differs from intel's definition for denormals and nans. In the functions we actually call the const version, we don't have such problems because those have standard float conventions.
Tracking issue: rust-lang/rust#149298
closes: #1076
(This uses
stdarch_const_x86for the intrinsics only, andstdarch_const_helpersfor everything else, so we can go for aarch64 next)Ik the diffcount is a lot, but most of it is just boilerplate. It is easier to review on a per-commit basis, and the following commits contain the "interesting" bits (others are just soulless adding of
constkeyword everywhere)reduce_addandreduce_mulintrinsics to usesimd_reduce_add_orderedandsimd_ordered_mul_ordered, respectively. As integer algebra is associative, it doesn't matter for correctness, and LLVM will use the most performant arrangement. The reason is that rustc_const_eval only implements these ordered reduction intrinsics, unordered reduction seems to be tricky to implement in const_eval)#[simd_test]quite a bit, adds a new "no arguments" mode to ease testing of intrinsics that require no target features and adds support for const testing)