Allow safely creating SIMD level tokens from #[target_feature] functions - #293
Conversation
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
Looks good to me! It's good to see it taken over with better results - the tests are a fantastic idea. I don't think its possible to test the other way around though, right?
I'm not overly happy with the name new_unchecked for the methods, but it doesn't block.
| #[inline] | ||
| pub const unsafe fn new_unchecked() -> Self { | ||
| #[target_feature(enable = "avx2,bmi1,bmi2,cmpxchg16b,f16c,fma,fxsr,lzcnt,movbe,popcnt,xsave")] | ||
| pub const fn new_unchecked() -> Self { |
There was a problem hiding this comment.
Does the name still need to be new_unchecked?
There was a problem hiding this comment.
Yeah I'm not thrilled about new_unchecked() either. Simply new() feels too low-hanging, doesn't communicate it's not very commonly used, but we have the docs pointing to Level::new() so it should be fine? We could also call it assume_supported().
There was a problem hiding this comment.
I've renamed the function to assume_supported() after sleeping on it for a few days.
2 participants
An updated version of #223
Also adds tests that verify that tokens can be safely made inside
kernel!macros to verify that the target_feature annotations match.