Drop support for very old compilers and simplify accordingly #345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
ebiggers
commented
Mar 1, 2024
•
ebiggers
commented
- Drop support for very old versions of gcc and clang
- Get rid of COMPILER_SUPPORTS_TARGET_FUNCTION_ATTRIBUTE
- common_defs.h: fix docs for __has_attribute and __has_builtin
- common_defs.h: simplify checks for very old gcc versions
- lib/arm: simplify checks for very old clang versions
- lib/x86: centralize the intrinsic header inclusions
- lib/x86: remove a workaround for very old gcc versions
- lib/x86: get rid of HAVE_TARGET_INTRINSICS
- lib/x86: simplify checks for very old gcc and clang versions
- lib/x86: simplify by not forcing *_INTRIN on when *_NATIVE
- lib/x86: simplify by not trying to skip target attributes
- lib/x86: remove unnecessary NATIVE macros
- lib/x86: simplify checks for intrinsics
- lib/x86: adding missing MSVC and Apple clang version checks
- lib: make lists of CPU feature bits easier to read
ebiggers
force-pushed
the
dev
branch
7 times, most recently
from
82ff676
to
3aea1f3
Compare
Drop support for gcc before v4.9 and clang before v3.9. This will allow simplifying some code. For example, we'll be able to assume that if the compiler is gcc or clang, then on x86 the target function attribute will work properly and intrinsics up to AVX2 will be supported. Document and start explicitly enforcing the minimum compiler versions. For MSVC, start enforcing a minimum of Visual Studio 2015. However, I believe this was already required due to the use of stdbool.h.
COMPILER_SUPPORTS_TARGET_FUNCTION_ATTRIBUTE is being misused to check not just whether the target function attribute is supported, but also whether inline assembly is supported (for cpuid). Meanwhile, we no longer support gcc versions that lack support for the target function attribute, which simplifies things somewhat. Therefore, replace COMPILER_SUPPORTS_TARGET_FUNCTION_ATTRIBUTE with simply checking __GNUC__ || __clang__ directly.
Since we no longer support gcc or clang versions that lack the target function attribute or bswap builtins, replace the corresponding gcc version checks with simply __GNUC__. (__GNUC__ means gcc-compatible, not gcc per se, but this is fine.)
Since we now only support clang 3.9 and later, the checks for clang 3.4 and clang 3.5 can simply use defined(__clang__).
On all the compilers that we support using x86 intrinsics with, we no longer support very old versions that don't have immintrin.h. Therefore, just include immintrin.h from lib/x86/cpu_features.h, making it available to all code in lib/x86/. Handle the workaround for clang in MSVC compatibility mode in the same place.
The bug that the EBX_CONSTRAINT hack was working around was fixed in gcc 4.6, and our minimum gcc version is now 4.9.
Since we no longer support the very old versions of gcc or clang where x86 intrinsics don't work properly, the HAVE_TARGET_INTRINSICS macro is always true when the compiler is gcc, clang, or MSVC. As a result, it's now redundant with other conditions checked, except for HAVE_SSE2_INTRIN. Just remove it.
Since we've bumped up the minimum gcc and clang versions to 4.9 and 3.9 respectively, when checking for support for intrinsics that were already present in those versions simply use __GNUC__ || __clang__. (__GNUC__ means gcc-compatible, not gcc per se, but this is fine.)
It's probably safe to assume that if e.g. __AVX2__ is defined then the AVX2 intrinsics are available. However, it doesn't seem worthwhile for the code to check for this because there's always already a compiler version check for using the intrinsics which is sufficient by itself.
It's true that if e.g. __AVX2__ is defined then it's unnecessary to use
__attribute__((target("avx2"))) on functions that use AVX-2 intrinsics.
But it seems to be harmless to add it. All the target options we use on
x86 seem to work additively, so at worst it's a no-op. We also no
longer support any gcc or clang versions that don't support the target
function attribute. Therefore, simplify the code by not skipping using
the target function attribute when all the options are already enabled
in the main target.
Note that this change has no effect on MSVC builds, as MSVC doesn't
support the target function attribute anyway.
Since most of the uses of the HAVE_*_NATIVE macros have been removed, and many of them provide no additional value over the original compiler-provided macro like __AVX2__ anyway, there's not much point in having them anymore. Remove them, except for HAVE_SSE2_NATIVE and HAVE_BMI2_NATIVE which are still worthwhile to have.
Since all the x86 HAVE_*_INTRIN values are now, in practice, determined by compiler version checks, it's simpler to just fold these checks directly into the code that actually uses the intrinsics (*_impl.h). This allows many of the conditions to be merged together. This approach does mean that the conditions for using a particular type of intrinsic may get duplicated in different places as more functions get added. This is partly why I used the layer of indirection originally. But in practice this hasn't been much of a problem. For crc32_x86_vpclmulqdq_avx2() we also have to check the compiler version anyway because MSVC supports both VPCLMULQDQ and AVX2, but not together.
Most of the recently added code that uses newer x86 intrinsics (AVX-512, VPCLMULQDQ, AVX-VNNI) is being compiled unconditionally when the compiler is MSVC or Apple clang. This is inconsistent with the minimum versions of these compilers that we claim to support. Therefore, add the needed checks against _MSC_VER and __apple_build_version__.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.