| Message ID | 20220624201036.3740866-1-goldstein.w.n@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] x86: Fix backwards Prefer_No_VZEROUPPER check in ifunc-evex.h | expand |
On Fri, Jun 24, 2022 at 1:10 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > Add third argument to X86_ISA_CPU_FEATURES_ARCH_P macro so the runtime > CPU_FEATURES_ARCH_P check can be inverted if the > MINIMUM_X86_ISA_LEVEL is not high enough to constantly evaluate > the check. > > Use this new macro to correct the backwards check in ifunc-evex.h > --- > sysdeps/x86/isa-ifunc-macros.h | 29 +++++++++++++++++++++------ > sysdeps/x86/isa-level.h | 26 +++++++++--------------- > sysdeps/x86_64/multiarch/ifunc-evex.h | 4 ++-- > 3 files changed, 35 insertions(+), 24 deletions(-) > > diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h > index ba6826d518..a3c98c841c 100644 > --- a/sysdeps/x86/isa-ifunc-macros.h > +++ b/sysdeps/x86/isa-ifunc-macros.h > @@ -56,15 +56,32 @@ > # define X86_IFUNC_IMPL_ADD_V1(...) > #endif > > -#define X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED(name) \ > - ((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) > +/* Both X86_ISA_CPU_FEATURE_USABLE_P and X86_ISA_CPU_FEATURES_ARCH_P > + should only be used to check if a condition is true. I.e: > + > + if (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Good > + if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Bad If (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) works, if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) should also work. > + > + There should be no need for inverting USABLE_P checks, but there is > + often need for inverting ARCH_P checks. If you want to get the not > + of an ARCH_P feature do: > + > + if (X86_ISA_CPU_FEATURES_ARCH_P (..., !)) // Good > + */ > + > > #define X86_ISA_CPU_FEATURE_USABLE_P(ptr, name) \ > - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ > || CPU_FEATURE_USABLE_P (ptr, name)) > > -#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name) \ > - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > - || CPU_FEATURES_ARCH_P (ptr, name)) > + > +/* When using X86_ISA_CPU_FEATURES_ARCH_P a third argument must be > + provided to optionally invert the runtime CPU_FEATURES_ARCH_P > + check. This is so we can consistently constant-evaluate conditions > + using Feature_X86_ISA_LEVEL <= MINIMUM_X86_ISA_LEVEL. */ > +#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name, not) \ > + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ > + || not CPU_FEATURES_ARCH_P (ptr, name)) > + > > #endif > diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h > index 7cae11c228..bad9aba099 100644 > --- a/sysdeps/x86/isa-level.h > +++ b/sysdeps/x86/isa-level.h > @@ -65,12 +65,8 @@ > (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4) > > > -/* > - * CPU Features that are hard coded as enabled depending on ISA build > - * level. > - * - Values > 0 features are always ENABLED if: > - * Value >= MINIMUM_X86_ISA_LEVEL > - */ > +/* CPU Features that are default set depending on ISA build level. > + Feature is assumed set if: Value <= MINIMUM_X86_ISA_LEVEL. */ This isn't accurate for Prefer_No_VZEROUPPER_X86_ISA_LEVEL. I think this should be removed. Each feature needs a comment to describe the default. > > /* ISA level >= 4 guaranteed includes. */ > @@ -81,18 +77,16 @@ > #define AVX2_X86_ISA_LEVEL 3 > #define BMI2_X86_ISA_LEVEL 3 > > -/* > - * NB: This may not be fully assumable for ISA level >= 3. From > - * looking over the architectures supported in cpu-features.h the > - * following CPUs may have an issue with this being default set: > - * - AMD Excavator > - */ > +/* NB: This feature is enabled when ISA level >= 3, which was disabled > + for the following CPUs: > + - AMD Excavator > + when ISA level < 3. */ > #define AVX_Fast_Unaligned_Load_X86_ISA_LEVEL 3 > > -/* > - * KNL (the only cpu that sets this supported in cpu-features.h) > - * builds with ISA V1 so this shouldn't harm any architectures. > - */ > +/* NB: This feature is disabled when ISA level >= 3, which was enabled > + for the following CPUs: > + - Intel KNL > + when ISA level < 3. */ > #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 > > #define ISA_SHOULD_BUILD(isa_build_level) \ > diff --git a/sysdeps/x86_64/multiarch/ifunc-evex.h b/sysdeps/x86_64/multiarch/ifunc-evex.h > index 856c6261f8..310cfd269f 100644 > --- a/sysdeps/x86_64/multiarch/ifunc-evex.h > +++ b/sysdeps/x86_64/multiarch/ifunc-evex.h > @@ -37,7 +37,7 @@ IFUNC_SELECTOR (void) > if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX2) > && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, BMI2) > && X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, > - AVX_Fast_Unaligned_Load)) > + AVX_Fast_Unaligned_Load, )) > { > if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512VL) > && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512BW)) > @@ -52,7 +52,7 @@ IFUNC_SELECTOR (void) > return OPTIMIZE (avx2_rtm); > > if (X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, > - Prefer_No_VZEROUPPER)) > + Prefer_No_VZEROUPPER, !)) > return OPTIMIZE (avx2); > } > > -- > 2.34.1 >
On Fri, Jun 24, 2022 at 1:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Fri, Jun 24, 2022 at 1:10 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > Add third argument to X86_ISA_CPU_FEATURES_ARCH_P macro so the runtime > > CPU_FEATURES_ARCH_P check can be inverted if the > > MINIMUM_X86_ISA_LEVEL is not high enough to constantly evaluate > > the check. > > > > Use this new macro to correct the backwards check in ifunc-evex.h > > --- > > sysdeps/x86/isa-ifunc-macros.h | 29 +++++++++++++++++++++------ > > sysdeps/x86/isa-level.h | 26 +++++++++--------------- > > sysdeps/x86_64/multiarch/ifunc-evex.h | 4 ++-- > > 3 files changed, 35 insertions(+), 24 deletions(-) > > > > diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h > > index ba6826d518..a3c98c841c 100644 > > --- a/sysdeps/x86/isa-ifunc-macros.h > > +++ b/sysdeps/x86/isa-ifunc-macros.h > > @@ -56,15 +56,32 @@ > > # define X86_IFUNC_IMPL_ADD_V1(...) > > #endif > > > > -#define X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED(name) \ > > - ((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) > > +/* Both X86_ISA_CPU_FEATURE_USABLE_P and X86_ISA_CPU_FEATURES_ARCH_P > > + should only be used to check if a condition is true. I.e: > > + > > + if (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Good > > + if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Bad > > If (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) works, > if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) should also > work. > > > + > > + There should be no need for inverting USABLE_P checks, but there is > > + often need for inverting ARCH_P checks. If you want to get the not > > + of an ARCH_P feature do: > > + > > + if (X86_ISA_CPU_FEATURES_ARCH_P (..., !)) // Good > > + */ > > + > > > > #define X86_ISA_CPU_FEATURE_USABLE_P(ptr, name) \ > > - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ > > || CPU_FEATURE_USABLE_P (ptr, name)) > > > > -#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name) \ > > - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > - || CPU_FEATURES_ARCH_P (ptr, name)) > > + > > +/* When using X86_ISA_CPU_FEATURES_ARCH_P a third argument must be > > + provided to optionally invert the runtime CPU_FEATURES_ARCH_P > > + check. This is so we can consistently constant-evaluate conditions > > + using Feature_X86_ISA_LEVEL <= MINIMUM_X86_ISA_LEVEL. */ > > +#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name, not) \ > > + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ > > + || not CPU_FEATURES_ARCH_P (ptr, name)) > > + > > > > #endif > > diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h > > index 7cae11c228..bad9aba099 100644 > > --- a/sysdeps/x86/isa-level.h > > +++ b/sysdeps/x86/isa-level.h > > @@ -65,12 +65,8 @@ > > (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4) > > > > > > -/* > > - * CPU Features that are hard coded as enabled depending on ISA build > > - * level. > > - * - Values > 0 features are always ENABLED if: > > - * Value >= MINIMUM_X86_ISA_LEVEL > > - */ > > +/* CPU Features that are default set depending on ISA build level. > > + Feature is assumed set if: Value <= MINIMUM_X86_ISA_LEVEL. */ > > This isn't accurate for Prefer_No_VZEROUPPER_X86_ISA_LEVEL. > I think this should be removed. Each feature needs a comment to > describe the default. How about: /* Depending on ISA level some feature checks will default evaluate to true if the MINIMUM_X86_ISA_LEVEL is high enough. The check on a feature will default evaluate to true if: Value <= MINIMUM_X86_ISA_LEVEL. */ ? > > > > > /* ISA level >= 4 guaranteed includes. */ > > @@ -81,18 +77,16 @@ > > #define AVX2_X86_ISA_LEVEL 3 > > #define BMI2_X86_ISA_LEVEL 3 > > > > -/* > > - * NB: This may not be fully assumable for ISA level >= 3. From > > - * looking over the architectures supported in cpu-features.h the > > - * following CPUs may have an issue with this being default set: > > - * - AMD Excavator > > - */ > > +/* NB: This feature is enabled when ISA level >= 3, which was disabled > > + for the following CPUs: > > + - AMD Excavator > > + when ISA level < 3. */ > > #define AVX_Fast_Unaligned_Load_X86_ISA_LEVEL 3 > > > > -/* > > - * KNL (the only cpu that sets this supported in cpu-features.h) > > - * builds with ISA V1 so this shouldn't harm any architectures. > > - */ > > +/* NB: This feature is disabled when ISA level >= 3, which was enabled > > + for the following CPUs: > > + - Intel KNL > > + when ISA level < 3. */ > > #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 > > > > #define ISA_SHOULD_BUILD(isa_build_level) \ > > diff --git a/sysdeps/x86_64/multiarch/ifunc-evex.h b/sysdeps/x86_64/multiarch/ifunc-evex.h > > index 856c6261f8..310cfd269f 100644 > > --- a/sysdeps/x86_64/multiarch/ifunc-evex.h > > +++ b/sysdeps/x86_64/multiarch/ifunc-evex.h > > @@ -37,7 +37,7 @@ IFUNC_SELECTOR (void) > > if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX2) > > && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, BMI2) > > && X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, > > - AVX_Fast_Unaligned_Load)) > > + AVX_Fast_Unaligned_Load, )) > > { > > if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512VL) > > && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512BW)) > > @@ -52,7 +52,7 @@ IFUNC_SELECTOR (void) > > return OPTIMIZE (avx2_rtm); > > > > if (X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, > > - Prefer_No_VZEROUPPER)) > > + Prefer_No_VZEROUPPER, !)) > > return OPTIMIZE (avx2); > > } > > > > -- > > 2.34.1 > > > > > -- > H.J.
On Fri, Jun 24, 2022 at 2:26 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > On Fri, Jun 24, 2022 at 1:33 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Fri, Jun 24, 2022 at 1:10 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > > > Add third argument to X86_ISA_CPU_FEATURES_ARCH_P macro so the runtime > > > CPU_FEATURES_ARCH_P check can be inverted if the > > > MINIMUM_X86_ISA_LEVEL is not high enough to constantly evaluate > > > the check. > > > > > > Use this new macro to correct the backwards check in ifunc-evex.h > > > --- > > > sysdeps/x86/isa-ifunc-macros.h | 29 +++++++++++++++++++++------ > > > sysdeps/x86/isa-level.h | 26 +++++++++--------------- > > > sysdeps/x86_64/multiarch/ifunc-evex.h | 4 ++-- > > > 3 files changed, 35 insertions(+), 24 deletions(-) > > > > > > diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h > > > index ba6826d518..a3c98c841c 100644 > > > --- a/sysdeps/x86/isa-ifunc-macros.h > > > +++ b/sysdeps/x86/isa-ifunc-macros.h > > > @@ -56,15 +56,32 @@ > > > # define X86_IFUNC_IMPL_ADD_V1(...) > > > #endif > > > > > > -#define X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED(name) \ > > > - ((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) > > > +/* Both X86_ISA_CPU_FEATURE_USABLE_P and X86_ISA_CPU_FEATURES_ARCH_P > > > + should only be used to check if a condition is true. I.e: > > > + > > > + if (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Good > > > + if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Bad > > > > If (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) works, > > if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) should also > > work. > > > > > + > > > + There should be no need for inverting USABLE_P checks, but there is > > > + often need for inverting ARCH_P checks. If you want to get the not > > > + of an ARCH_P feature do: > > > + > > > + if (X86_ISA_CPU_FEATURES_ARCH_P (..., !)) // Good > > > + */ > > > + > > > > > > #define X86_ISA_CPU_FEATURE_USABLE_P(ptr, name) \ > > > - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > > + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ > > > || CPU_FEATURE_USABLE_P (ptr, name)) > > > > > > -#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name) \ > > > - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > > - || CPU_FEATURES_ARCH_P (ptr, name)) > > > + > > > +/* When using X86_ISA_CPU_FEATURES_ARCH_P a third argument must be > > > + provided to optionally invert the runtime CPU_FEATURES_ARCH_P > > > + check. This is so we can consistently constant-evaluate conditions > > > + using Feature_X86_ISA_LEVEL <= MINIMUM_X86_ISA_LEVEL. */ > > > +#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name, not) \ > > > + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ > > > + || not CPU_FEATURES_ARCH_P (ptr, name)) > > > + > > > > > > #endif > > > diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h > > > index 7cae11c228..bad9aba099 100644 > > > --- a/sysdeps/x86/isa-level.h > > > +++ b/sysdeps/x86/isa-level.h > > > @@ -65,12 +65,8 @@ > > > (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4) > > > > > > > > > -/* > > > - * CPU Features that are hard coded as enabled depending on ISA build > > > - * level. > > > - * - Values > 0 features are always ENABLED if: > > > - * Value >= MINIMUM_X86_ISA_LEVEL > > > - */ > > > +/* CPU Features that are default set depending on ISA build level. > > > + Feature is assumed set if: Value <= MINIMUM_X86_ISA_LEVEL. */ > > > > This isn't accurate for Prefer_No_VZEROUPPER_X86_ISA_LEVEL. > > I think this should be removed. Each feature needs a comment to > > describe the default. > > How about: > > /* Depending on ISA level some feature checks will default evaluate > to true if the MINIMUM_X86_ISA_LEVEL is high enough. The check > on a feature will default evaluate to true if: > Value <= MINIMUM_X86_ISA_LEVEL. */ Depending on the minimum ISA level, a feature check result can be a compile-time constant. True or false may be confusing since the meaning of the compile-time constant depends on the feature. > ? > > > > > > > > > /* ISA level >= 4 guaranteed includes. */ > > > @@ -81,18 +77,16 @@ > > > #define AVX2_X86_ISA_LEVEL 3 > > > #define BMI2_X86_ISA_LEVEL 3 > > > > > > -/* > > > - * NB: This may not be fully assumable for ISA level >= 3. From > > > - * looking over the architectures supported in cpu-features.h the > > > - * following CPUs may have an issue with this being default set: > > > - * - AMD Excavator > > > - */ > > > +/* NB: This feature is enabled when ISA level >= 3, which was disabled > > > + for the following CPUs: > > > + - AMD Excavator > > > + when ISA level < 3. */ > > > #define AVX_Fast_Unaligned_Load_X86_ISA_LEVEL 3 > > > > > > -/* > > > - * KNL (the only cpu that sets this supported in cpu-features.h) > > > - * builds with ISA V1 so this shouldn't harm any architectures. > > > - */ > > > +/* NB: This feature is disabled when ISA level >= 3, which was enabled > > > + for the following CPUs: > > > + - Intel KNL > > > + when ISA level < 3. */ > > > #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 > > > > > > #define ISA_SHOULD_BUILD(isa_build_level) \ > > > diff --git a/sysdeps/x86_64/multiarch/ifunc-evex.h b/sysdeps/x86_64/multiarch/ifunc-evex.h > > > index 856c6261f8..310cfd269f 100644 > > > --- a/sysdeps/x86_64/multiarch/ifunc-evex.h > > > +++ b/sysdeps/x86_64/multiarch/ifunc-evex.h > > > @@ -37,7 +37,7 @@ IFUNC_SELECTOR (void) > > > if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX2) > > > && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, BMI2) > > > && X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, > > > - AVX_Fast_Unaligned_Load)) > > > + AVX_Fast_Unaligned_Load, )) > > > { > > > if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512VL) > > > && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512BW)) > > > @@ -52,7 +52,7 @@ IFUNC_SELECTOR (void) > > > return OPTIMIZE (avx2_rtm); > > > > > > if (X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, > > > - Prefer_No_VZEROUPPER)) > > > + Prefer_No_VZEROUPPER, !)) > > > return OPTIMIZE (avx2); > > > } > > > > > > -- > > > 2.34.1 > > > > > > > > > -- > > H.J.
diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h index ba6826d518..a3c98c841c 100644 --- a/sysdeps/x86/isa-ifunc-macros.h +++ b/sysdeps/x86/isa-ifunc-macros.h @@ -56,15 +56,32 @@ # define X86_IFUNC_IMPL_ADD_V1(...) #endif -#define X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED(name) \ - ((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) +/* Both X86_ISA_CPU_FEATURE_USABLE_P and X86_ISA_CPU_FEATURES_ARCH_P + should only be used to check if a condition is true. I.e: + + if (X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Good + if (!X86_ISA_CPU_FEATURE{S}_{USABLE|ARCH}_P (...)) // Bad + + There should be no need for inverting USABLE_P checks, but there is + often need for inverting ARCH_P checks. If you want to get the not + of an ARCH_P feature do: + + if (X86_ISA_CPU_FEATURES_ARCH_P (..., !)) // Good + */ + #define X86_ISA_CPU_FEATURE_USABLE_P(ptr, name) \ - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ || CPU_FEATURE_USABLE_P (ptr, name)) -#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name) \ - (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ - || CPU_FEATURES_ARCH_P (ptr, name)) + +/* When using X86_ISA_CPU_FEATURES_ARCH_P a third argument must be + provided to optionally invert the runtime CPU_FEATURES_ARCH_P + check. This is so we can consistently constant-evaluate conditions + using Feature_X86_ISA_LEVEL <= MINIMUM_X86_ISA_LEVEL. */ +#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name, not) \ + (((name##_X86_ISA_LEVEL) <= MINIMUM_X86_ISA_LEVEL) \ + || not CPU_FEATURES_ARCH_P (ptr, name)) + #endif diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h index 7cae11c228..bad9aba099 100644 --- a/sysdeps/x86/isa-level.h +++ b/sysdeps/x86/isa-level.h @@ -65,12 +65,8 @@ (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4) -/* - * CPU Features that are hard coded as enabled depending on ISA build - * level. - * - Values > 0 features are always ENABLED if: - * Value >= MINIMUM_X86_ISA_LEVEL - */ +/* CPU Features that are default set depending on ISA build level. + Feature is assumed set if: Value <= MINIMUM_X86_ISA_LEVEL. */ /* ISA level >= 4 guaranteed includes. */ @@ -81,18 +77,16 @@ #define AVX2_X86_ISA_LEVEL 3 #define BMI2_X86_ISA_LEVEL 3 -/* - * NB: This may not be fully assumable for ISA level >= 3. From - * looking over the architectures supported in cpu-features.h the - * following CPUs may have an issue with this being default set: - * - AMD Excavator - */ +/* NB: This feature is enabled when ISA level >= 3, which was disabled + for the following CPUs: + - AMD Excavator + when ISA level < 3. */ #define AVX_Fast_Unaligned_Load_X86_ISA_LEVEL 3 -/* - * KNL (the only cpu that sets this supported in cpu-features.h) - * builds with ISA V1 so this shouldn't harm any architectures. - */ +/* NB: This feature is disabled when ISA level >= 3, which was enabled + for the following CPUs: + - Intel KNL + when ISA level < 3. */ #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 #define ISA_SHOULD_BUILD(isa_build_level) \ diff --git a/sysdeps/x86_64/multiarch/ifunc-evex.h b/sysdeps/x86_64/multiarch/ifunc-evex.h index 856c6261f8..310cfd269f 100644 --- a/sysdeps/x86_64/multiarch/ifunc-evex.h +++ b/sysdeps/x86_64/multiarch/ifunc-evex.h @@ -37,7 +37,7 @@ IFUNC_SELECTOR (void) if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX2) && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, BMI2) && X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, - AVX_Fast_Unaligned_Load)) + AVX_Fast_Unaligned_Load, )) { if (X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512VL) && X86_ISA_CPU_FEATURE_USABLE_P (cpu_features, AVX512BW)) @@ -52,7 +52,7 @@ IFUNC_SELECTOR (void) return OPTIMIZE (avx2_rtm); if (X86_ISA_CPU_FEATURES_ARCH_P (cpu_features, - Prefer_No_VZEROUPPER)) + Prefer_No_VZEROUPPER, !)) return OPTIMIZE (avx2); }