Initial conversion to const generics #1018
Conversation
|
@Amanieu: no appropriate reviewer found, use r? to override |
|
I can help reviewing this later today. |
Amanieu
force-pushed
the
const_generics
branch
from
9b6911c
to
0c3a550
Compare
| #[rustc_legacy_const_generics(2)] | ||
| #[stable(feature = "simd_x86", since = "1.27.0")] | ||
| pub unsafe fn _mm_shuffle_ps<const mask: i32>(a: __m128, b: __m128) -> __m128 { | ||
| assert!(mask >= 0 && mask <= 255); |
There was a problem hiding this comment.
Technically this is a breaking change compared to before, where you just discarded bits outside the lowest 8.
There was a problem hiding this comment.
True, but C compilers already reject out-of-range immediates at compile-time. I would argue that this is a bug fix since we model the vendor's API more closely.
There was a problem hiding this comment.
Checking the Intel Intrinsics Guide again, I see that it's supposed to be unsigned int imm8, so you're right.
Why did we ever accept an i32 to begin with? I guess this should have been u32 the whole time, and arguably u8.
There was a problem hiding this comment.
oof, i'm actually already subscribed to this one and i'd forgotten about it. okay then.
Actually since _MM_SHUFFLE is still nightly, maybe we should change the shuffle to correctly take u32 and then change _MM_SHUFFLE (again) to make it also emit u32 values.
But even then, if we're going to panic on values outside of u8, it's hard to justify not just taking a u8 to begin with. I guess, are these conversions to const generics supposed to be otherwise completely non-breaking?
There was a problem hiding this comment.
rust-lang/rust#82447 will rewrite an expression at the AST level to use const generics, but won't do anything to change the type of that expression. So I don't think we can change the type here, unless we decide to accept the breaking change anyways.
There was a problem hiding this comment.
Well if the ship has sailed that's ultimately fine I guess.
| @@ -101,7 +109,23 @@ fn functions(input: TokenStream, dirs: &[&str]) -> TokenStream { | |||
| } else { | |||
| quote! { None } | |||
| }; | |||
| let required_const = find_required_const(&f.attrs); | |||
|
|
|||
| let required_const = find_required_const("rustc_args_required_const", &f.attrs); | |||
This PR builds on top rust-lang/rust#82447 to start transition intrinsics to use const generics. This is the first step towards solving #248.
_mm_shuffle_psand_mm_prefetchto use const generics.