Message ID | 20220624063653.2126416-3-goldstein.w.n@gmail.com |
---|---|
State | New |
Headers | show |
Series | [v1,1/7] x86: Align entry for memrchr to 64-bytes. | expand |
On Thu, Jun 23, 2022 at 11:37 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > Some ARCH_P features such as Prefer_No_VZEROUPPER used to disable > implementations if true as opposed fo the majority of features > such as AVX2 which are used to enabled features. > > Different ISA build levels want override certain disabling > features. For example ISA build level >= 3 should ignore > Prefer_No_VZEROUPPER which means converting the check to > false (as opposed to true for a feature like AVX2). > --- > sysdeps/x86/isa-ifunc-macros.h | 4 ++++ > sysdeps/x86/isa-level.h | 7 ++++--- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h > index ba6826d518..e229e612a4 100644 > --- a/sysdeps/x86/isa-ifunc-macros.h > +++ b/sysdeps/x86/isa-ifunc-macros.h > @@ -67,4 +67,8 @@ > (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > || CPU_FEATURES_ARCH_P (ptr, name)) > > +#define X86_ISA_CPU_FEATURES_ARCH_P_NOT(ptr, name) \ > + (!X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > + && !CPU_FEATURES_ARCH_P (ptr, name)) > + > #endif > diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h > index 7cae11c228..e1a30ed83e 100644 > --- a/sysdeps/x86/isa-level.h > +++ b/sysdeps/x86/isa-level.h > @@ -66,10 +66,10 @@ > > > /* > - * CPU Features that are hard coded as enabled depending on ISA build > - * level. > + * CPU Features that are hard coded as enabled/disabled depending on > + * ISA build level. > * - Values > 0 features are always ENABLED if: > - * Value >= MINIMUM_X86_ISA_LEVEL > + * Value <= MINIMUM_X86_ISA_LEVEL > */ > > > @@ -92,6 +92,7 @@ > /* > * KNL (the only cpu that sets this supported in cpu-features.h) > * builds with ISA V1 so this shouldn't harm any architectures. > + * NB: Only use this feature with the ARCH_P_NOT macro. > */ > #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 > > -- > 2.34.1 > This is a bug fix. Please make it a separate patch set. Please also send individual patches, instead of a set, if there are no dependencies between patches. Thanks.
On Fri, Jun 24, 2022 at 7:32 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jun 23, 2022 at 11:37 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > Some ARCH_P features such as Prefer_No_VZEROUPPER used to disable > > implementations if true as opposed fo the majority of features > > such as AVX2 which are used to enabled features. > > > > Different ISA build levels want override certain disabling > > features. For example ISA build level >= 3 should ignore > > Prefer_No_VZEROUPPER which means converting the check to > > false (as opposed to true for a feature like AVX2). > > --- > > sysdeps/x86/isa-ifunc-macros.h | 4 ++++ > > sysdeps/x86/isa-level.h | 7 ++++--- > > 2 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h > > index ba6826d518..e229e612a4 100644 > > --- a/sysdeps/x86/isa-ifunc-macros.h > > +++ b/sysdeps/x86/isa-ifunc-macros.h > > @@ -67,4 +67,8 @@ > > (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > || CPU_FEATURES_ARCH_P (ptr, name)) > > > > +#define X86_ISA_CPU_FEATURES_ARCH_P_NOT(ptr, name) \ > > + (!X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > + && !CPU_FEATURES_ARCH_P (ptr, name)) > > + > > #endif > > diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h > > index 7cae11c228..e1a30ed83e 100644 > > --- a/sysdeps/x86/isa-level.h > > +++ b/sysdeps/x86/isa-level.h > > @@ -66,10 +66,10 @@ > > > > > > /* > > - * CPU Features that are hard coded as enabled depending on ISA build > > - * level. > > + * CPU Features that are hard coded as enabled/disabled depending on > > + * ISA build level. > > * - Values > 0 features are always ENABLED if: > > - * Value >= MINIMUM_X86_ISA_LEVEL > > + * Value <= MINIMUM_X86_ISA_LEVEL > > */ > > > > > > @@ -92,6 +92,7 @@ > > /* > > * KNL (the only cpu that sets this supported in cpu-features.h) > > * builds with ISA V1 so this shouldn't harm any architectures. > > + * NB: Only use this feature with the ARCH_P_NOT macro. > > */ > > #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 > > > > -- > > 2.34.1 > > > > This is a bug fix. Please make it a separate patch set. Please also > send individual patches, > instead of a set, if there are no dependencies between patches. > How about something like this? diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h index ba6826d518..108d9cf023 100644 --- a/sysdeps/x86/isa-ifunc-macros.h +++ b/sysdeps/x86/isa-ifunc-macros.h @@ -63,8 +63,8 @@ (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ || 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)) +#define X86_ISA_CPU_FEATURES_ARCH_P(ptr, name, not) \ + (not (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ + || CPU_FEATURES_ARCH_P (ptr, name))) #endif diff --git a/sysdeps/x86_64/multiarch/ifunc-evex.h b/sysdeps/x86_64/multiarch/ifunc-evex.h index 856c6261f8..2d59e022f0 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); }
On Fri, Jun 24, 2022 at 7:33 AM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Thu, Jun 23, 2022 at 11:37 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > > > Some ARCH_P features such as Prefer_No_VZEROUPPER used to disable > > implementations if true as opposed fo the majority of features > > such as AVX2 which are used to enabled features. > > > > Different ISA build levels want override certain disabling > > features. For example ISA build level >= 3 should ignore > > Prefer_No_VZEROUPPER which means converting the check to > > false (as opposed to true for a feature like AVX2). > > --- > > sysdeps/x86/isa-ifunc-macros.h | 4 ++++ > > sysdeps/x86/isa-level.h | 7 ++++--- > > 2 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h > > index ba6826d518..e229e612a4 100644 > > --- a/sysdeps/x86/isa-ifunc-macros.h > > +++ b/sysdeps/x86/isa-ifunc-macros.h > > @@ -67,4 +67,8 @@ > > (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > || CPU_FEATURES_ARCH_P (ptr, name)) > > > > +#define X86_ISA_CPU_FEATURES_ARCH_P_NOT(ptr, name) \ > > + (!X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ > > + && !CPU_FEATURES_ARCH_P (ptr, name)) > > + > > #endif > > diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h > > index 7cae11c228..e1a30ed83e 100644 > > --- a/sysdeps/x86/isa-level.h > > +++ b/sysdeps/x86/isa-level.h > > @@ -66,10 +66,10 @@ > > > > > > /* > > - * CPU Features that are hard coded as enabled depending on ISA build > > - * level. > > + * CPU Features that are hard coded as enabled/disabled depending on > > + * ISA build level. > > * - Values > 0 features are always ENABLED if: > > - * Value >= MINIMUM_X86_ISA_LEVEL > > + * Value <= MINIMUM_X86_ISA_LEVEL > > */ > > > > > > @@ -92,6 +92,7 @@ > > /* > > * KNL (the only cpu that sets this supported in cpu-features.h) > > * builds with ISA V1 so this shouldn't harm any architectures. > > + * NB: Only use this feature with the ARCH_P_NOT macro. > > */ > > #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3 > > > > -- > > 2.34.1 > > > > This is a bug fix. Please make it a separate patch set. Please also > send individual patches, > instead of a set, if there are no dependencies between patches. Split patches. This and the change to ifunc-evex in series. Rest standalone. > > Thanks. > > -- > H.J.
diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h index ba6826d518..e229e612a4 100644 --- a/sysdeps/x86/isa-ifunc-macros.h +++ b/sysdeps/x86/isa-ifunc-macros.h @@ -67,4 +67,8 @@ (X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ || CPU_FEATURES_ARCH_P (ptr, name)) +#define X86_ISA_CPU_FEATURES_ARCH_P_NOT(ptr, name) \ + (!X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED (name) \ + && !CPU_FEATURES_ARCH_P (ptr, name)) + #endif diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h index 7cae11c228..e1a30ed83e 100644 --- a/sysdeps/x86/isa-level.h +++ b/sysdeps/x86/isa-level.h @@ -66,10 +66,10 @@ /* - * CPU Features that are hard coded as enabled depending on ISA build - * level. + * CPU Features that are hard coded as enabled/disabled depending on + * ISA build level. * - Values > 0 features are always ENABLED if: - * Value >= MINIMUM_X86_ISA_LEVEL + * Value <= MINIMUM_X86_ISA_LEVEL */ @@ -92,6 +92,7 @@ /* * KNL (the only cpu that sets this supported in cpu-features.h) * builds with ISA V1 so this shouldn't harm any architectures. + * NB: Only use this feature with the ARCH_P_NOT macro. */ #define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3