BUG: verify the OS supports avx instruction #10814
Conversation
|
Nice! Have you filed a bug with gcc? I guess they should fix |
| npy_cpu_supports(const char * feature) | ||
| { | ||
| #ifdef HAVE___BUILTIN_CPU_SUPPORTS | ||
| if (strcmp(feature, "avx2") == 0) { |
There was a problem hiding this comment.
Why not npy_cpu_supports_avx() and npy_cpu_supports_avx2(), rather than incurring a strcmp?
There was a problem hiding this comment.
Would probably make sense, but we only use it during module load so performance is not really an issue, with link time optimization the compiler would also fold it away.
|
yes I have filed a GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85100 |
numpy/core/src/umath/cpuid.c
Outdated
| #endif | ||
| } | ||
| else if (strcmp(feature, "avx") == 0) { | ||
| #ifdef HAVE_ATTRIBUTE_TARGET_AVX |
There was a problem hiding this comment.
Why repeat this here when it's already used at the call site?
There was a problem hiding this comment.
indeed this is unnecessary, fixed
There was a problem hiding this comment.
I think they were needed for gcc versions that didn't know about avx/avx2, though those should not be around anymore
On some systems you can disable avx registers but the gcc builtin does only checks if the cpu has the feature. Before using avx functions check the OS support with xgetbv. Closes numpygh-10787 Closes numpygh-9534
juliantaylor
force-pushed
the
improve-cpuid
branch
from
58cbc7b
to
cde5583
Compare
|
Thanks Julian. |
|
GCC bug now fixed, for what it's worth: |
| #if HAVE_XGETBV | ||
| /* | ||
| * use bytes for xgetbv to avoid issues with compiler not knowing the | ||
| * instruction |
There was a problem hiding this comment.
This comment doesn't look true?
On some systems you can disable avx registers but the gcc builtin does
only checks if the cpu has the feature.
Before using avx functions check the OS support with xgetbv.
Closes gh-10787
Closes gh-9534