Skip to content
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

SIMDUTF_CAN_ALWAYS_RUN_* macros broken #391

Closed
jakubjelinek opened this issue Apr 5, 2024 · 2 comments
Closed

SIMDUTF_CAN_ALWAYS_RUN_* macros broken #391

jakubjelinek opened this issue Apr 5, 2024 · 2 comments

Comments

@jakubjelinek
Copy link

As mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=2273542#c14 the SIMDUTF_CAN_ALWAYS_RUN_{HASWELL,ICELAKE,WESTMERE} macros are broken at least for GCC.
This is because GCC (since ~4.7 when not using -save-temps, since 14 when preprocessing separately from compilation) on #pragma GCC target ISA changes undefines/defines the predefined preprocessor macros like __AVX2__, so that they reflect the current set of the enabled/disabled ISAs in the region.
When you

#define SIMDUTF_CAN_ALWAYS_RUN_AVX2 __AVX2__
#if SIMDUTF_CAN_ALWAYS_RUN_AVX2
#else
#pragma GCC push_options
#pragma GCC target ("avx2")
#endif
...
#if SIMDUTF_CAN_ALWAYS_RUN_AVX2
#else
#pragma GCC pop_options
#endif

and compile this with -mno-avx2, the first SIMDUTF_CAN_ALWAYS_RUN_AVX2 use will evaluate to 0 because __AVX2__ is not defined, but then #pragma GCC target ("avx2") defines it, on the second use it will evaluate to 1 because __AVX2__ is defined to 1 there and finally #pragma GCC pop_options will undefine it again.
The code clearly expects it to evaluate the same in both cases, otherwise there is a misbalance, which in the rhbz case causes unsupported_implementation to be compiled with avx2,bmi,lzcnt,popcnt isas.

I'd suggest either change macros like:

#define SIMDUTF_CAN_ALWAYS_RUN_HASWELL ((SIMDUTF_IMPLEMENTATION_HASWELL) && (SIMDUTF_IS_X86_64) && (__AVX2__))

to

#if ((SIMDUTF_IMPLEMENTATION_HASWELL) && (SIMDUTF_IS_X86_64) && (__AVX2__))
#define SIMDUTF_CAN_ALWAYS_RUN_HASWELL 1
#else
#define SIMDUTF_CAN_ALWAYS_RUN_HASWELL 0
#endif

Then it will have a constant value through the whole compilation unit.

@jakubjelinek
Copy link
Author

What GCC normally uses in its intrinsic headers is:

#ifndef __AVX2__
#pragma GCC push_options
#pragma GCC target("avx2")
#define __DISABLE_AVX2__
#endif /* __AVX2__ */

at the start and

#ifdef __DISABLE_AVX2__
#undef __DISABLE_AVX2__
#pragma GCC pop_options
#endif /* __DISABLE_AVX2__ */

at the end.

@lemire
Copy link
Member

lemire commented Apr 6, 2024

Solved with last release. Thanks.

@lemire lemire closed this as completed Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants