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

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Wed, Feb 28, 2024 at 11:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> When glibc is built with ISA level 3 or above enabled, SSE resolvers
> aren't available and glibc fails to build:
>
> ld: .../elf/librtld.os: in function `init_cpu_features':
> .../elf/../sysdeps/x86/cpu-features.c:1200:(.text+0x1445f): undefined reference to `_dl_runtime_resolve_fxsave'
> ld: .../elf/librtld.os: relocation R_X86_64_PC32 against undefined hidden symbol `_dl_runtime_resolve_fxsave' can not be used when making a shared object
> /usr/local/bin/ld: final link failed: bad value
>
> For ISA level 3 or above, don't use _dl_runtime_resolve_fxsave nor
> _dl_tlsdesc_dynamic_fxsave and also exclude _dl_tlsdesc_dynamic_fxsave.
>
> This fixes BZ #31429.
> ---
>  sysdeps/x86/cpu-features.c  | 17 +++++++++++------
>  sysdeps/x86_64/dl-tlsdesc.S | 15 +++++++++------
>  2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index d71e8d3d2e..e7c7ece462 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -18,6 +18,7 @@
>
>  #include <dl-hwcap.h>
>  #include <libc-pointer-arith.h>
> +#include <isa-level.h>
>  #include <get-isa-level.h>
>  #include <cacheinfo.h>
>  #include <dl-cacheinfo.h>
> @@ -1195,7 +1196,9 @@ 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 (CPU_FEATURE_USABLE_P (cpu_features, XSAVEC))
>         {
> @@ -1216,22 +1219,24 @@ 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
>      }
> +#endif
>
>  #ifdef SHARED
>  # ifdef __x86_64__
> diff --git a/sysdeps/x86_64/dl-tlsdesc.S b/sysdeps/x86_64/dl-tlsdesc.S
> index ea69f5223a..057a10862a 100644
> --- a/sysdeps/x86_64/dl-tlsdesc.S
> +++ b/sysdeps/x86_64/dl-tlsdesc.S
> @@ -20,6 +20,7 @@
>  #include <tls.h>
>  #include <cpu-features-offsets.h>
>  #include <features-offsets.h>
> +#include <isa-level.h>
>  #include "tlsdesc.h"
>  #include "dl-trampoline-save.h"
>
> @@ -79,12 +80,14 @@ _dl_tlsdesc_undefweak:
>         .size   _dl_tlsdesc_undefweak, .-_dl_tlsdesc_undefweak
>
>  #ifdef SHARED
> -# define USE_FXSAVE
> -# define STATE_SAVE_ALIGNMENT  16
> -# define _dl_tlsdesc_dynamic   _dl_tlsdesc_dynamic_fxsave
> -# include "dl-tlsdesc-dynamic.h"
> -# undef _dl_tlsdesc_dynamic
> -# undef USE_FXSAVE
> +# if MINIMUM_X86_ISA_LEVEL < AVX_X86_ISA_LEVEL
> +#  define USE_FXSAVE
> +#  define STATE_SAVE_ALIGNMENT 16
> +#  define _dl_tlsdesc_dynamic  _dl_tlsdesc_dynamic_fxsave
> +#  include "dl-tlsdesc-dynamic.h"
> +#  undef _dl_tlsdesc_dynamic
> +#  undef USE_FXSAVE
> +# endif
>
>  # define USE_XSAVE
>  # define STATE_SAVE_ALIGNMENT  64
> --
> 2.43.2
>

LGTM.
Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>