Re: [PATCH] x86-64: Simplify minimum ISA check ifdef conditional with if

["H.J. Lu" <hjl.tools@gmail.com>]

On Sun, Mar 3, 2024 at 1:46 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Mar 1, 2024 at 4:40 PM Sunil K Pandey <skpgkp2@gmail.com> wrote:
> >
> > Replace minimum ISA check ifdef conditional with if.  Since
> > MINIMUM_X86_ISA_LEVEL and AVX_X86_ISA_LEVEL are compile time constants,
> > compiler will perform constant folding optimization getting same result.
> > ---
> >  sysdeps/x86/cpu-features.c | 19 ++++++++-----------
> >  1 file changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> > index e7c7ece462..4ea373dffa 100644
> > --- a/sysdeps/x86/cpu-features.c
> > +++ b/sysdeps/x86/cpu-features.c
> > @@ -1196,9 +1196,8 @@ no_cpuid:
> >                TUNABLE_CALLBACK (set_x86_shstk));
> >  #endif
> >
> > -#if MINIMUM_X86_ISA_LEVEL < AVX_X86_ISA_LEVEL
> > -  if (GLRO(dl_x86_cpu_features).xsave_state_size != 0)
> > -#endif
> > +  if (MINIMUM_X86_ISA_LEVEL >= AVX_X86_ISA_LEVEL
> > +      || (GLRO(dl_x86_cpu_features).xsave_state_size != 0))
> >      {
> >        if (CPU_FEATURE_USABLE_P (cpu_features, XSAVEC))
> >         {
> > @@ -1219,24 +1218,22 @@ no_cpuid:
> >  #endif
> >         }
> >      }
> > -#if MINIMUM_X86_ISA_LEVEL < AVX_X86_ISA_LEVEL
> >    else
> >      {
> > -# ifdef __x86_64__
> > +#ifdef __x86_64__
> >        GLRO(dl_x86_64_runtime_resolve) = _dl_runtime_resolve_fxsave;
> > -#  ifdef SHARED
> > +# ifdef SHARED
> >        GLRO(dl_x86_tlsdesc_dynamic) = _dl_tlsdesc_dynamic_fxsave;
> > -#  endif
> > -# else
> > -#  ifdef SHARED
> > +# endif
> > +#else
> > +# ifdef SHARED
> >        if (CPU_FEATURE_USABLE_P (cpu_features, FXSR))
> >         GLRO(dl_x86_tlsdesc_dynamic) = _dl_tlsdesc_dynamic_fxsave;
> >        else
> >         GLRO(dl_x86_tlsdesc_dynamic) = _dl_tlsdesc_dynamic_fnsave;
> > -#  endif
> >  # endif
> > -    }
> >  #endif
> > +    }
> >
> >  #ifdef SHARED
> >  # ifdef __x86_64__
> > --
> > 2.43.0
> >
>
> LGTM.
>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> Thanks.
>
> --
> H.J.

Should it also be backported to glibc 2.39 together with

commit befe2d3c4dec8be2cdd01a47132e47bdb7020922
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Feb 28 09:51:14 2024 -0800

    x86-64: Don't use SSE resolvers for ISA level 3 or above


-- 
H.J.