Consider limiting #[rustc_args_required_const(...)] to intrinsics.

[eddyb]

I've just been made aware that functions like this exist: https://github.com/rust-lang/stdarch/blob/abe96ca3b87fcca6aa1dfcefd40d8c8d92d2e673/crates/core_arch/src/x86_64/avx2.rs#L28-L34

#[rustc_args_required_const(1)]
// This intrinsic has no corresponding instruction.
#[stable(feature = "simd_x86", since = "1.27.0")]
pub unsafe fn _mm256_extract_epi64(a: __m256i, imm8: i32) -> i64 {
    let imm8 = (imm8 & 3) as u32;
    simd_extract(a.as_i64x4(), imm8)
}

I was under the impression that #[rustc_args_required_const(...)] would be used primarily for intrinsics (including the behind-rustc's-back imported LLVM intrinsics), and maybe unstable implementation details.

At least right now I am uncomfortable letting #[rustc_args_required_const(...)] proliferate any further, and would much prefer any stabilized library-defined functions that rely on the attribute, to become true intrinsics.

Sadly, there are a few of them, so this won't be trivial.

cc @rust-lang/compiler

[RalfJung]

Why does _mm256_extract_epi64 even use that attribute? simd_extract doesn't have it, so it does not seem relevant for codegen?

[hanna-kruppe]

While it's implemented using an LLVM IR instruction that supports variable indices, the corresponding C intrinsics and AVX instructions require an immediate, i.e., a compile time constant index. Variable indices on that target with that vector type lower to "store the vector to the stack and load the desired element from it" instead. So the attribute is useful for consistency with C, to avoid surprisingly bad code generation, and to avoid reliance on LLVM specifics.

[workingjubilee]

From the perspective of users of the relevant Intel and ARM APIs here, these are expected to be intrinsic functions, even though they are effectively provided as a library. So, parsing this request is a bit challenging. What is the desired outcome here?

[oli-obk]

Library APIs for users should be using const generics ideally. We could even make the intrinsics use const generics, but I think some APIs with #[rustc_args_required_const] have already been stabilized and are being used.

[workingjubilee]

That is correct, as seen in the code you quoted, these were stabilized in #49664 in 1.27 i.e. Rust 2018 with const generic stabilization not in sight and after passing a final comment period in #48556. They had their rustc_args_required_const adornment added at the same time as they were added,

That means these APIs have parameters that ask for const values that can be turned into immediates in a form that conforms well with the expected API of the Intel intrinsic functions in C and C++ compilers, as we can see here:
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm256_extract_epi64

So I imagine it's a very "drop in" experience for someone who is used to Intel intrinsics, but is... Admittedly Kinda Awkward in Rust terms.

[oli-obk]

Note that we can remove the rustc_args_required_const from https://github.com/rust-lang/stdarch/blob/9142a8a22d9ab4470167187e81aec503c2d70f60/crates/core_arch/src/x86_64/sse41.rs#L17-L25 and similar without that being any breaking change at all. To best of my knowledge this is true for all library functions, since the constness of argument, isn't "propagated" into the body of the function. So the only reason to make it require constness is to force users to use constants there, even though it's not necessary at all. Or are these library functions direct intrinsics on some platforms?

[RalfJung]

@oli-obk in #77477, some people are suggesting the intrinsics wrapped by those functions should also use rustc_args_required_const.

[oli-obk]

Sure, I thought they already had that. All the calls to these intrinsics use actual literal integer constants, I just want the library function to not have that attribute, as it does not forward its constant argument to the inner intrinsics, it does some runtime check on it.

[workingjubilee]

So the only reason to make it require constness is to force users to use constants there, even though it's not necessary at all.

So, my understanding, partly informed by @Lokathor's explanation at #73542 (comment), is that this is because it is matching the design of Intel's API. The input to that intrinsic function must be a value that can be compiled as an x86 asm immediate (expressed in the intrinsic function in Intel's guide as const int index, ). Then Intel's design for the intrinsic will only look at the lower bits. The & operation there, I believe, is done to make sure it actually only provides those lower bits. I don't know for sure, since I did not write these, but I don't think it's supposed to be a runtime check at all... I think it's done with the intent that the entire thing is const folded.

[Lokathor]

Yes that's also my understanding. These intrinsics aren't truly accepting const args, they're actually a high level approximation of immediate values being encoded into an assembly instruction.

It's not breaking to Rust to suddenly allow a varying argument in a future release, but then it would absolutely fail to do the one job it's supposed to do.

Rust is unfortunately bad at expressing a few low level concepts, this is one of them.

[RalfJung]

But this sounds like once const generics are sufficiently far along, we could provide new APIs based on const generics, deprecate the old ones, remove their rustc_args_required_const, and that would allow us to do the desired implementation cleanup inside the compiler?

Basically I am not sure if keeping rustc_args_required_const around when it basically just serves the purpose of a lint is worth it.

[workingjubilee]

given rust-lang/stdarch#876 (comment)

IMO we should panic on out-of-range immediates, with the intention of turning those into compile-time errors once we have enough support for const generics (and #[rustc_args_required_const] becomes syntax sugar for const generics).

cc @Amanieu

[Lokathor]

Yeah, once const generics can handle this they're basically the more "rusty" way to do all this.

However, this is actually farther along the dev timeline than min_const_generics last I checked. You'd need to be able to say something like "fail the build if the generic value is more than 3 bits".

[oli-obk]

cc @lcnr ^

[lcnr]

However, this is actually farther along the dev timeline than min_const_generics last I checked. You'd need to be able to say something like "fail the build if the generic value is more than 3 bits".

That requires us to be comfortable using a limited set of const_evaluatable_checked in std. I don't yet know how much work is left until this is the case, but expect it to be a lot easier than stabilizing it 😆

[Alexendoo]

rustc_args_required_const has been removed - #85110