-
Notifications
You must be signed in to change notification settings - Fork 505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Missing AVX2 version of Classic McEliece #896
Comments
I tried modifying the copy_from_upstream.yml file to pull in all of the Classic McEliece schemes, but got some weird behaviour. Here's an excerpt from the modified .yml file: kems:
-
name: classic_mceliece
default_implementation: vec
upstream_location: pqclean
schemes:
-
scheme: "348864"
pqclean_scheme: mceliece348864
pretty_name_full: Classic-McEliece-348864 When running copy_from_upstream.py, I get the following warnings output to the console:
And the following C code is produced, which, if you notice, is not valid. OQS_API OQS_STATUS OQS_KEM_classic_mceliece_348864_keypair(uint8_t *public_key, uint8_t *secret_key) {
#if defined(OQS_ENABLE_KEM_classic_mceliece_348864_clean)
#if defined(OQS_PORTABLE_BUILD)
OQS_CPU_EXTENSIONS available_cpu_extensions = OQS_get_available_CPU_extensions();
if () {
#endif /* OQS_PORTABLE_BUILD */
return (OQS_STATUS) PQCLEAN_MCELIECE348864_CLEAN_crypto_kem_keypair(public_key, secret_key);
#if defined(OQS_PORTABLE_BUILD)
} else {
return (OQS_STATUS) PQCLEAN_MCELIECE348864_VEC_crypto_kem_keypair(public_key, secret_key);
}
#endif /* OQS_PORTABLE_BUILD */
#else
#if defined(OQS_ENABLE_KEM_classic_mceliece_348864_sse)
#if defined(OQS_PORTABLE_BUILD)
OQS_CPU_EXTENSIONS available_cpu_extensions = OQS_get_available_CPU_extensions();
if (available_cpu_extensions.SSE4_1_ENABLED && available_cpu_extensions.POPCNT_ENABLED) {
#endif /* OQS_PORTABLE_BUILD */
return (OQS_STATUS) PQCLEAN_MCELIECE348864_SSE_crypto_kem_keypair(public_key, secret_key);
#if defined(OQS_PORTABLE_BUILD)
} else {
return (OQS_STATUS) PQCLEAN_MCELIECE348864_VEC_crypto_kem_keypair(public_key, secret_key);
}
#endif /* OQS_PORTABLE_BUILD */
#else
#if defined(OQS_ENABLE_KEM_classic_mceliece_348864_avx)
#if defined(OQS_PORTABLE_BUILD)
OQS_CPU_EXTENSIONS available_cpu_extensions = OQS_get_available_CPU_extensions();
if (available_cpu_extensions.AVX2_ENABLED && available_cpu_extensions.POPCNT_ENABLED) {
#endif /* OQS_PORTABLE_BUILD */
return (OQS_STATUS) PQCLEAN_MCELIECE348864_AVX_crypto_kem_keypair(public_key, secret_key);
#if defined(OQS_PORTABLE_BUILD)
} else {
return (OQS_STATUS) PQCLEAN_MCELIECE348864_VEC_crypto_kem_keypair(public_key, secret_key);
}
#endif /* OQS_PORTABLE_BUILD */
#else
return (OQS_STATUS) PQCLEAN_MCELIECE348864_VEC_crypto_kem_keypair(public_key, secret_key);
#endif
} Although there are four implementations available in PQClean (clean, vec, sse, avx2), I think our desired behaviour is: use avx2 if available, else use sse if available, else use vec. I'm not sure how to express that to copy_from_upstream. |
Is anyone already working this issue? When looking at it also noticed that a presumably idempotent |
I haven't worked on it since raising the issue with the comment above. |
FYI, #920 shall close this issue. One caveat: This is a "quick fix" in that it only enables import of "vec" and "avx" variants of McEliece; "sse2" and "clean" variants are explicitly suppressed: a) @bhess and @baentsch don't see much value in those variants; b) Supporting multiple optimized variants would make the "portability logic" very convoluted. Anyone disagreeing with this logic, please either comment (negatively) on #920 and/or create a "future-work" item for multi-variant, portable CPU feature support. |
Those choices makes sense to me, Michael: AVX2 has the highest performance of the non-portable ones, and vec has the highest performance of the portable ones. |
We only pull in the vec (vectorized plain C) implementation of Classic McEliece from PQClean; we should also pull in the avx2 implementation.
The text was updated successfully, but these errors were encountered: