Remove all uses of #[rustc_args_required_const]

[Amanieu]

To solve #248 we are moving away from #[rustc_args_required_const] and switching to using the recently stabilized const generics instead. The goal is to massively reduce the binary size of libcore.rlib which is currently bloated due to the massive match statements needed to properly handle constant arguments for intrinsics.

Instructions

1. Move constant arguments to const generics.

2. Change the #[rustc_args_required_const(N)] attribute to #[rustc_legacy_const_generics(N)]. The N arguments to the attribute remains the same. This attribute allows legacy code to continue calling this function by passing constants as a parameter. The legacy support will likely be deprecated in Rust 2021.

3. Use the static_assert! macro to perform bounds checking on the value. This is necessary because passing invalid value to intrinsics tends to trigger assert failures in LLVM if they reach codegen. Simply using assert! or match is not enough since they don't prevent the intrinsic from reaching codegen.

4. Eliminate unneeded constify_*/shuffle macros and pass the constant directly to the underlying LLVM intrinsic.

5. Update tests to call the function using const generics. Note that #[rustc_legacy_const_generics] does not work for functions defined in the same crate.

Example

Before

#[inline]
#[target_feature(enable = "sse")]
#[cfg_attr(test, assert_instr(shufps, mask = 3))]
#[rustc_args_required_const(2)]
#[stable(feature = "simd_x86", since = "1.27.0")]
pub unsafe fn _mm_shuffle_ps(a: __m128, b: __m128, mask: i32) -> __m128 {
    let mask = (mask & 0xFF) as u8;

    macro_rules! shuffle_done {
        ($x01:expr, $x23:expr, $x45:expr, $x67:expr) => {
            simd_shuffle4(a, b, [$x01, $x23, $x45, $x67])
        };
    }
    macro_rules! shuffle_x67 {
        ($x01:expr, $x23:expr, $x45:expr) => {
            match (mask >> 6) & 0b11 {
                0b00 => shuffle_done!($x01, $x23, $x45, 4),
                0b01 => shuffle_done!($x01, $x23, $x45, 5),
                0b10 => shuffle_done!($x01, $x23, $x45, 6),
                _ => shuffle_done!($x01, $x23, $x45, 7),
            }
        };
    }
    macro_rules! shuffle_x45 {
        ($x01:expr, $x23:expr) => {
            match (mask >> 4) & 0b11 {
                0b00 => shuffle_x67!($x01, $x23, 4),
                0b01 => shuffle_x67!($x01, $x23, 5),
                0b10 => shuffle_x67!($x01, $x23, 6),
                _ => shuffle_x67!($x01, $x23, 7),
            }
        };
    }
    macro_rules! shuffle_x23 {
        ($x01:expr) => {
            match (mask >> 2) & 0b11 {
                0b00 => shuffle_x45!($x01, 0),
                0b01 => shuffle_x45!($x01, 1),
                0b10 => shuffle_x45!($x01, 2),
                _ => shuffle_x45!($x01, 3),
            }
        };
    }
    match mask & 0b11 {
        0b00 => shuffle_x23!(0),
        0b01 => shuffle_x23!(1),
        0b10 => shuffle_x23!(2),
        _ => shuffle_x23!(3),
    }
}

After

#[inline]
#[target_feature(enable = "sse")]
#[cfg_attr(test, assert_instr(shufps, mask = 3))]
#[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 {
    static_assert!(mask: i32 where mask >= 0 && mask <= 255);
    simd_shuffle4(
        a,
        b,
        [
            mask as u32 & 0b11,
            (mask as u32 >> 2) & 0b11,
            ((mask as u32 >> 4) & 0b11) + 4,
            ((mask as u32 >> 6) & 0b11) + 4,
        ],
    )
}

[Lokathor]

Question: Should we continue to accept all possible i32 values and simply use mask&0xFF as was done before to ensure that the value is in range?

rather than the new static assert that is

[Amanieu]

I think we should use static_assert to ensure the value is in range. This is what C compilers do, so it makes sense to match that API.

[minybot]

I will start from avx512f then avx512bw....
However, the orig is _mm_shuffle_ps(a, b, int imm8) and the new one is _mm_shuffle_ps::(a,b).
Is there any way to make it "look" like _mm_shuffle_ps(a,b, <>) or make the func is still 3 input parameters?

[Lokathor]

i think the attribute makes rustc magically rewrite the 3 arg version to the const generic + 2 arg version. So callers can still continue to use the 3 arg version.

[minybot]

i think the attribute makes rustc magically rewrite the 3 arg version to the const generic + 2 arg version. So callers can still continue to use the 3 arg version.

I just tried it. It seems 3 arg one does not work.

error[E0061]: this function takes 2 arguments but 3 arguments were supplied
--> crates/core_arch/src/x86/sse.rs:2948:17
|
2948 | let r = _mm_shuffle_ps(a, b, 5);
| ^^^^^^^^^^^^^^ - - - supplied 3 arguments
| |
| expected 2 arguments

[lqd]

This is explained in point 6 above: the attribute rewrites the calls from other crates, not the current one where the tests are. The tests need to be rewritten to match the new signature with the const generic immediates.

@minybot the most recent merged PRs have some other examples of changes to intrinsics and their tests, in case the OP and instructions are still unclear

[minybot]

Thanks. This solution looks great.

[minybot]

pub unsafe fn _mm_shuffle_ps(a: __m128, b: __m128) -> __m128 {
if I have 2 constants, should I put <const c1: i32; const c2: i32> ?

[Amanieu]

Yes.

[Amanieu]

All uses of #[rustc_args_required_const] are now gone! The only ones remaining are on simd_shuffle but that's fine since that's what the attribute was originally meant for.

Many thanks to everyone involved who contributed to this!

[RalfJung]

Wow that's amazing. :)

Does that mean that we can remove rustc_args_require_const from the compiler?

EDIT: Oh, you said it is still used for some shuffle intrinsics. How is that "fine"? The attribute was not originally meant for any stable API, it was meant as a temporary hack (as far as I recall).

[Amanieu]

It's used on simd_shuffle in platform-intrinsics. This is not a public API.

Actually I'm not sure if it's even required any more. I'll have to try removing it to see if the tests still pass.

[RalfJung]

This is not a public API.

Ah, that's good. :) Then I hope we can find some other, non-promotion-based, solution.

[Amanieu]

I've removed the last uses in #1113.