support avx512 without nightly

[Slixe]

Remove the requirements of nightly for AVX512 in build.rs & #[cfg]

Solve #758

[Slixe]

this would actually require to set 1.89 as MSRV.

[tarcieri]

Ideally it'd be nice to get a release out with a 1.85 MSRV so it could get packaged on Debian stable.

@Slixe perhaps it could still require an explicit cfg opt-in, to avoid the MSRV bump?

We could then later bump MSRV in a point release and remove it.

[Slixe]

@tarcieri could it work if we simply enable the auto detection for avx512 >= 1.89 only from build.rs ?

[tarcieri]

Yeah, you could temporarily add rust version detection until we bump MSRV

[tarcieri]

@Slixe seems like you can remove draft?

[Slixe]

I'm still having an issue, I'm not sure how the conditional works.

Compiling criterion v0.5.1        
warning: unexpected `cfg` condition value: `avx512ifma,avx512vl`      
  --> curve25519-dalek/src/backend/vector/scalar_mul/pippenger.rs:12:1
   |                                                                                             
12 | / #[curve25519_dalek_derive::unsafe_target_feature_specialize(
13 | |     "avx2",                
14 | |     conditional("avx512ifma,avx512vl", curve25519_dalek_backend = "avx512")
   | |_____________________________________^
   |                                                                                             
   = note: expected values for `target_feature` are: `10e60`, `2e3`, `32s`, `3e3r1`, `3e3r2`, `3e3r3`, `3e7`, `7e10`, `a`, `aclass`, `adx`, `aes`, `altivec`, `alu32`, `amx-avx512`, `amx-bf16`, `
amx-complex`, `amx-fp16`, `amx-fp8`, `amx-int8`, `amx-movrs`, `amx-tf32`, `amx-tile`, `amx-transpose`, `apxf`, `atomics`, `avx`, `avx10.1`, `avx10.2`, `avx2`, `avx512bf16`, `avx512bitalg`, `avx5
12bw`, `avx512cd`, and `avx512dq` and 429 more
   = note: using a cfg inside a attribute macro will use the cfgs from the destination crate and not the ones from the defining crate
   = help: try referring to `curve25519_dalek_derive::unsafe_target_feature_specialize` crate for guidance on how handle this unexpected cfg
   = help: the attribute macro `curve25519_dalek_derive::unsafe_target_feature_specialize` may come from an old version of the `curve25519_dalek_derive` crate, try updating your dependency with 
`cargo update -p curve25519_dalek_derive`   
   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
   = note: requested on the command line with `-W unexpected-cfgs`
   = note: this warning originates in the attribute macro `curve25519_dalek_derive::unsafe_target_feature_specialize` (in Nightly builds, run with -Z macro-backtrace for more info)
                                                

rustc 1.91.1

EDIT: it seems target_feature support one by one only, and the macro don't split it correctly?

[tarcieri]

I think we could probably completely get rid of curve25519-dalek-derive, although it might require a bump to Rust 1.86: #744

[Slixe]

I believe this new commit should be good enough?
avx512 backend is only enabled if both features are available anyway.
and this prevent the bump of MSRV as you asked

[Slixe]

@tarcieri the avx512 backend may be available & detected but won't work if the target features aren't included for compilation.

which option would you prefer?

• enables it in build.rs only when available

• in the code, add the requirement of target features along the actual avx512 backend check like here:

    #[cfg(curve25519_dalek_backend = "avx512")]
    Avx512ifma(

[tarcieri]

@Slixe I'm not sure I quite understand the problem. If all the AVX-512 code is feature-gated with #[cfg(curve25519_dalek_backend = "avx512")], then it shouldn't be an issue to declare #[(unsafe_)target_feature("avx512*")] immediately after that.

If the code isn't compiled with #[cfg(curve25519_dalek_backend = "avx512")], then conditional compilation should ensure that none of that code is included.

build.rs can detect when the rustc version and target architecture are compatible and set curve25519_dalek_backend = "avx512" accordingly.

Then cpufeatures is already configured to use CPUID to detect if the features are enabled on the target CPU at runtime, I believe.

[pinkforest]

There is no way to test this in CI. How / where does this get tested and who ensures it gets done if not in CI long run ?

Especially if this is default-on backend that should be tested in CI... ideally..

Even if the backend doesn't change the stuff around it just might and might be hard to catch errors there.

Or even how gating errors can lead to trouble e.g.:

• Incorrect ML-DSA output on Alpine Linux and Ampere Altra cryspen/libcrux#1220

[tarcieri]

I believe some of the GitHub Actions runners have AVX-512 support. If CPU feature detection is enabled, it should at least opportunistically test when the runner has support. Not a great answer, but it is what it is:

actions/runner#1069

Otherwise they suggest setting up a self-hosted runner. Intel SDE is also an option, I believe.

[pinkforest]

I believe some of the GitHub Actions runners have AVX-512 support.

They used to have that with intel - which we relied on earlier - to but not anymore and they have no AMD AVX512 either.

As I suggested in libcrux, should probably maybe prehaps validate that we test all the gating gi matrix as well

• Snapshot cfg permutations from simd backends #853

Anyways I would not turn it on by default - not because the backend hasn't changed but more that the gating is quite involved easy to mess up and it's not the first time it has caused trouble elsewhere and we can't test it.

[tarcieri]

This PR to add AVX-512 support to chacha20 includes a CI config that uses Intel SDE as the runner: https://github.com/RustCrypto/stream-ciphers/pull/477/files#diff-4406b424fd59f8f997267f1955fac9159be3d9ba9cde8f6b6b8a53c528edcba9