Re: [PATCH v4 1/2] x86: Add defines / utilities for making ISA specific x86 builds

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

On Tue, Jun 21, 2022 at 5:30 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> 1. Factor out some of the ISA level defines in isa-level.c to
>    standalone header isa-level.h
>
> 2. Add new headers with ISA level dependent macros for handling
>    ifuncs.
>
> Note, this file does not change any code.
>
> Tested with and without multiarch on x86_64 for ISA levels:
> {generic, x86-64-v2, x86-64-v3, x86-64-v4}
> ---
>  sysdeps/generic/ifunc-init.h         |   8 ++
>  sysdeps/x86/init-arch.h              |   5 +-
>  sysdeps/x86/isa-cpu-feature-checks.h |  60 +++++++++++++
>  sysdeps/x86/isa-ifunc-macros.h       | 130 +++++++++++++++++++++++++++
>  sysdeps/x86/isa-level.c              |  17 ++--
>  sysdeps/x86/isa-level.h              |  67 ++++++++++++++
>  sysdeps/x86_64/isa-default-impl.h    |  49 ++++++++++
>  7 files changed, 323 insertions(+), 13 deletions(-)
>  create mode 100644 sysdeps/x86/isa-cpu-feature-checks.h
>  create mode 100644 sysdeps/x86/isa-ifunc-macros.h
>  create mode 100644 sysdeps/x86/isa-level.h
>  create mode 100644 sysdeps/x86_64/isa-default-impl.h
>
> diff --git a/sysdeps/generic/ifunc-init.h b/sysdeps/generic/ifunc-init.h
> index 929e22ff5d..76f91c663c 100644
> --- a/sysdeps/generic/ifunc-init.h
> +++ b/sysdeps/generic/ifunc-init.h
> @@ -55,3 +55,11 @@
>  #define OPTIMIZE2(name)        EVALUATOR2 (SYMBOL_NAME, name)
>  /* Default is to use OPTIMIZE2.  */
>  #define OPTIMIZE(name) OPTIMIZE2(name)
> +
> +/* Syntactic sugar for common usage of the OPTIMIZE and OPTIMIZE1 macros
> +   respectively.  */
> +#define OPTIMIZE_DECL(...)                                                    \
> +  extern __typeof (REDIRECT_NAME) OPTIMIZE (__VA_ARGS__) attribute_hidden;
> +
> +#define OPTIMIZE_DECL1(...)                                                   \
> +  extern __typeof (REDIRECT_NAME) OPTIMIZE1 (__VA_ARGS__) attribute_hidden;

Unrelated.  Please remove them.

> diff --git a/sysdeps/x86/init-arch.h b/sysdeps/x86/init-arch.h
> index 277c15f116..a9fb4a1975 100644
> --- a/sysdeps/x86/init-arch.h
> +++ b/sysdeps/x86/init-arch.h
> @@ -19,7 +19,10 @@
>  #include <ifunc-init.h>
>  #include <isa.h>
>
> -#ifndef __x86_64__
> +#ifdef __x86_64__
> +# include <isa-ifunc-macros.h>
> +# include <isa-cpu-feature-checks.h>

Why add 2 header files?  Does one work?

> +#else
>  /* Due to the reordering and the other nifty extensions in i686, it is
>     not really good to use heavily i586 optimized code on an i686.  It's
>     better to use i486 code if it isn't an i586.  */
> diff --git a/sysdeps/x86/isa-cpu-feature-checks.h b/sysdeps/x86/isa-cpu-feature-checks.h
> new file mode 100644
> index 0000000000..44b77a8c1f
> --- /dev/null
> +++ b/sysdeps/x86/isa-cpu-feature-checks.h
> @@ -0,0 +1,60 @@
> +/* Common ifunc selection utils
> +   All versions must be listed in ifunc-impl-list.c.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _ISA_CPU_FEATURE_CHECKS_H
> +#define _ISA_CPU_FEATURE_CHECKS_H 1
> +
> +/*
> + * CPU Features that are hard coded enabled / disabled depending on ISA build

Do we disable an ISA feature?  We check if a CPU Feature
is included for a given ISA level.

> + *   level.
> + *    - Values > 0 features are always ENABLED if:
> + *          Value >= MINIMUM_X86_ISA_LEVEL
> + */
> +
> +#include <isa-level.h>
> +
> +/* ISA level >= 4 guaranteed includes.  */
> +#define AVX512VL_X86_ISA_LEVEL 4
> +#define AVX512BW_X86_ISA_LEVEL 4
> +
> +/* ISA level >= 3 guaranteed includes.  */
> +#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
> + */
> +#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.
> + */
> +#define Prefer_No_VZEROUPPER_X86_ISA_LEVEL 3
> +
> +/*
> + * ISA independent non-guaranteed includes.  Set value at 255 which is
> + * greater than any forseable ISA level.
> + */
> +#define RTM_X86_ISA_LEVEL 255

Do we really need to define the ISA level for RTM?

> +
> +#endif
> diff --git a/sysdeps/x86/isa-ifunc-macros.h b/sysdeps/x86/isa-ifunc-macros.h
> new file mode 100644
> index 0000000000..4c28a057c5
> --- /dev/null
> +++ b/sysdeps/x86/isa-ifunc-macros.h
> @@ -0,0 +1,130 @@
> +/* Common ifunc selection utils
> +   All versions must be listed in ifunc-impl-list.c.
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _ISA_IFUNC_MACROS_H
> +#define _ISA_IFUNC_MACROS_H 1
> +
> +#include <isa-level.h>
> +#include <sys/cdefs.h>
> +
> +/* Only include at the level of the minimum build ISA or higher. I.e
> +   if built with ISA=V1, then include all implementations. On the
> +   other hand if built with ISA=V3 only include V3/V4
> +   implementations. If there is no implementation at or above the
> +   minimum build ISA level, then include the highest ISA level
> +   implementation.  */
> +#if MINIMUM_X86_ISA_LEVEL <= 4
> +# define X86_IFUNC_IMPL_ADD_V4(...) IFUNC_IMPL_ADD (__VA_ARGS__)
> +# define return_X86_OPTIMIZE_V4(...) return OPTIMIZE (__VA_ARGS__)
> +# define return_X86_OPTIMIZE1_V4(...) return OPTIMIZE1 (__VA_ARGS__)
> +#endif
> +#if MINIMUM_X86_ISA_LEVEL <= 3
> +# define X86_IFUNC_IMPL_ADD_V3(...) IFUNC_IMPL_ADD (__VA_ARGS__)
> +# define return_X86_OPTIMIZE_V3(...) return OPTIMIZE (__VA_ARGS__)
> +# define return_X86_OPTIMIZE1_V3(...) return OPTIMIZE1 (__VA_ARGS__)
> +#endif
> +#if MINIMUM_X86_ISA_LEVEL <= 2
> +# define X86_IFUNC_IMPL_ADD_V2(...) IFUNC_IMPL_ADD (__VA_ARGS__)
> +# define return_X86_OPTIMIZE_V2(...) return OPTIMIZE (__VA_ARGS__)
> +# define return_X86_OPTIMIZE1_V2(...) return OPTIMIZE1 (__VA_ARGS__)
> +#endif
> +#if MINIMUM_X86_ISA_LEVEL <= 1
> +# define X86_IFUNC_IMPL_ADD_V1(...) IFUNC_IMPL_ADD (__VA_ARGS__)
> +# define return_X86_OPTIMIZE_V1(...) return OPTIMIZE (__VA_ARGS__)
> +# define return_X86_OPTIMIZE1_V1(...) return OPTIMIZE1 (__VA_ARGS__)
> +#endif
> +
> +#ifndef return_X86_OPTIMIZE_V4
> +# define X86_IFUNC_IMPL_ADD_V4(...)
> +# define return_X86_OPTIMIZE_V4(...) (void) (0)
> +# define return_X86_OPTIMIZE1_V4(...) (void) (0)
> +#endif
> +#ifndef return_X86_OPTIMIZE_V3
> +# define X86_IFUNC_IMPL_ADD_V3(...)
> +# define return_X86_OPTIMIZE_V3(...) (void) (0)
> +# define return_X86_OPTIMIZE1_V3(...) (void) (0)
> +#endif
> +#ifndef return_X86_OPTIMIZE_V2
> +# define X86_IFUNC_IMPL_ADD_V2(...)
> +# define return_X86_OPTIMIZE_V2(...) (void) (0)
> +# define return_X86_OPTIMIZE1_V2(...) (void) (0)
> +#endif
> +#ifndef return_X86_OPTIMIZE_V1
> +# define X86_IFUNC_IMPL_ADD_V1(...)
> +# define return_X86_OPTIMIZE_V1(...) (void) (0)
> +# define return_X86_OPTIMIZE1_V1(...) (void) (0)
> +#endif
> +
> +#if MINIMUM_X86_ISA_LEVEL == 1
> +# define X86_OPTIMIZE_FALLBACK(v1, ...) OPTIMIZE (v1)
> +#elif MINIMUM_X86_ISA_LEVEL == 2
> +# define X86_OPTIMIZE_FALLBACK(v1, v2, ...) OPTIMIZE (v2)
> +#elif MINIMUM_X86_ISA_LEVEL == 3
> +# define X86_OPTIMIZE_FALLBACK(v1, v2, v3, ...) OPTIMIZE (v3)
> +#elif MINIMUM_X86_ISA_LEVEL == 4
> +# define X86_OPTIMIZE_FALLBACK(v1, v2, v3, v4) OPTIMIZE (v4)

Do we need these? Shouldn't X86_ERROR_IF_REACHABLE be
sufficient?

> +#else
> +# error "Unsupported ISA Level"
> +#endif
> +
> +#if MINIMUM_X86_ISA_LEVEL >= 4
> +__errordecl (
> +    __unreachable_isa_above_4,
> +    "This code should be unreachable if ISA level >= 4 build ");
> +# define X86_ERROR_IF_REACHABLE_V4()                                   \
> +    __unreachable_isa_above_4 ();                                      \
> +    __builtin_unreachable ();

Do we need __builtin_unreachable here?  Compiler must
know it is unreachable.

> +#else
> +# define X86_ERROR_IF_REACHABLE_V4()
> +#endif
> +
> +#if MINIMUM_X86_ISA_LEVEL >= 3
> +__errordecl (__unreachable_isa_above_3,
> +            "This code should be unreachable if ISA level >= 3 build");
> +# define X86_ERROR_IF_REACHABLE_V3()                                   \
> +    __unreachable_isa_above_3 ();                                      \
> +    __builtin_unreachable ();
> +#else
> +# define X86_ERROR_IF_REACHABLE_V3()
> +#endif
> +
> +#if MINIMUM_X86_ISA_LEVEL >= 2
> +__errordecl (__unreachable_isa_above_2,
> +            "This code should be unreachable if ISA level >= 2 build");
> +# define X86_ERROR_IF_REACHABLE_V2()                                   \
> +    __unreachable_isa_above_2 ();                                      \
> +    __builtin_unreachable ();
> +#else
> +# define X86_ERROR_IF_REACHABLE_V2()
> +#endif
> +
> +#define X86_ISA_CPU_FEATURE_NAME(name) (name##_X86_ISA_LEVEL)

No need for this. This macro name is confusing.

> +
> +#define X86_ISA_CPU_FEATURE_CONST_CHECK_ENABLED(name)                  \
> +  (X86_ISA_CPU_FEATURE_NAME (name) <= MINIMUM_X86_ISA_LEVEL)

X86_ISA_CPU_FEATURE_INCLUDED?

> +#define X86_ISA_CPU_FEATURE_USABLE_P(ptr, name)                        \
> +  (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))
> +
> +#endif
> diff --git a/sysdeps/x86/isa-level.c b/sysdeps/x86/isa-level.c
> index 09cd72ab20..5b7a2da870 100644
> --- a/sysdeps/x86/isa-level.c
> +++ b/sysdeps/x86/isa-level.c
> @@ -26,38 +26,31 @@
>     <https://www.gnu.org/licenses/>.  */
>
>  #include <elf.h>
> -
> +#include <sysdeps/x86/isa-level.h>
>  /* ELF program property for x86 ISA level.  */
>  #ifdef INCLUDE_X86_ISA_LEVEL
> -# if defined __SSE__ && defined __SSE2__
> +# if MINIMUM_X86_ISA_LEVEL >= 1
>  /* NB: ISAs, excluding MMX, in x86-64 ISA level baseline are used.  */
>  #  define ISA_BASELINE GNU_PROPERTY_X86_ISA_1_BASELINE
>  # else
>  #  define ISA_BASELINE 0
>  # endif
>
> -# if ISA_BASELINE && defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
> -     && defined HAVE_X86_LAHF_SAHF && defined __POPCNT__ \
> -     && defined __SSE3__ && defined __SSSE3__ && defined __SSE4_1__ \
> -     && defined __SSE4_2__
> +# if MINIMUM_X86_ISA_LEVEL >= 2
>  /* NB: ISAs in x86-64 ISA level v2 are used.  */
>  #  define ISA_V2       GNU_PROPERTY_X86_ISA_1_V2
>  # else
>  #  define ISA_V2       0
>  # endif
>
> -# if ISA_V2 && defined __AVX__ && defined __AVX2__ && defined __F16C__ \
> -     && defined __FMA__ && defined __LZCNT__ && defined HAVE_X86_MOVBE \
> -     && defined __BMI__ && defined __BMI2__
> +# if MINIMUM_X86_ISA_LEVEL >= 3
>  /* NB: ISAs in x86-64 ISA level v3 are used.  */
>  #  define ISA_V3       GNU_PROPERTY_X86_ISA_1_V3
>  # else
>  #  define ISA_V3       0
>  # endif
>
> -# if ISA_V3 && defined __AVX512F__ && defined __AVX512BW__ \
> -     && defined __AVX512CD__ && defined __AVX512DQ__ \
> -     && defined __AVX512VL__
> +# if MINIMUM_X86_ISA_LEVEL >= 4
>  /* NB: ISAs in x86-64 ISA level v4 are used.  */
>  #  define ISA_V4       GNU_PROPERTY_X86_ISA_1_V4
>  # else
> diff --git a/sysdeps/x86/isa-level.h b/sysdeps/x86/isa-level.h
> new file mode 100644
> index 0000000000..33dec72bde
> --- /dev/null
> +++ b/sysdeps/x86/isa-level.h
> @@ -0,0 +1,67 @@
> +/* Header defining the minimum x86 ISA level
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   In addition to the permissions in the GNU Lesser General Public
> +   License, the Free Software Foundation gives you unlimited
> +   permission to link the compiled version of this file with other
> +   programs, and to distribute those programs without any restriction
> +   coming from the use of this file.  (The Lesser General Public
> +   License restrictions do apply in other respects; for example, they
> +   cover modification of the file, and distribution when not linked
> +   into another program.)
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _ISA_LEVEL_H
> +#define _ISA_LEVEL_H
> +
> +#if defined __SSE__ && defined __SSE2__
> +/* NB: ISAs, excluding MMX, in x86-64 ISA level baseline are used.  */
> +# define __X86_ISA_V1 1
> +#else
> +# define __X86_ISA_V1 0
> +#endif
> +
> +#if __X86_ISA_V1 && defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16               \
> +    && defined HAVE_X86_LAHF_SAHF && defined __POPCNT__ && defined __SSE3__   \
> +    && defined __SSSE3__ && defined __SSE4_1__ && defined __SSE4_2__
> +/* NB: ISAs in x86-64 ISA level v2 are used.  */
> +# define __X86_ISA_V2 1
> +#else
> +# define __X86_ISA_V2 0
> +#endif
> +
> +#if __X86_ISA_V2 && defined __AVX__ && defined __AVX2__ && defined __F16C__   \
> +    && defined __FMA__ && defined __LZCNT__ && defined HAVE_X86_MOVBE         \
> +    && defined __BMI__ && defined __BMI2__
> +/* NB: ISAs in x86-64 ISA level v3 are used.  */
> +# define __X86_ISA_V3 1
> +#else
> +# define __X86_ISA_V3 0
> +#endif
> +
> +#if __X86_ISA_V3 && defined __AVX512F__ && defined __AVX512BW__               \
> +    && defined __AVX512CD__ && defined __AVX512DQ__ && defined __AVX512VL__
> +/* NB: ISAs in x86-64 ISA level v4 are used.  */
> +# define __X86_ISA_V4 1
> +#else
> +# define __X86_ISA_V4 0
> +#endif
> +
> +#define MINIMUM_X86_ISA_LEVEL                                                 \
> +  (__X86_ISA_V1 + __X86_ISA_V2 + __X86_ISA_V3 + __X86_ISA_V4)
> +
> +#endif
> diff --git a/sysdeps/x86_64/isa-default-impl.h b/sysdeps/x86_64/isa-default-impl.h
> new file mode 100644
> index 0000000000..db0635c8e7
> --- /dev/null
> +++ b/sysdeps/x86_64/isa-default-impl.h
> @@ -0,0 +1,49 @@
> +/* Utility for including proper default function based on ISA level
> +   Copyright (C) 2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <isa-level.h>
> +
> +#ifndef DEFAULT_IMPL_V1
> +# error "Must have at least ISA V1 Version"
> +#endif
> +
> +#ifndef DEFAULT_IMPL_V2
> +# define DEFAULT_IMPL_V2 DEFAULT_IMPL_V1
> +#endif
> +
> +#ifndef DEFAULT_IMPL_V3
> +# define DEFAULT_IMPL_V3 DEFAULT_IMPL_V2
> +#endif
> +
> +#ifndef DEFAULT_IMPL_V4
> +# define DEFAULT_IMPL_V4 DEFAULT_IMPL_V3
> +#endif
> +
> +#if MINIMUM_X86_ISA_LEVEL == 1
> +# define ISA_DEFAULT_IMPL DEFAULT_IMPL_V1
> +#elif MINIMUM_X86_ISA_LEVEL == 2
> +# define ISA_DEFAULT_IMPL DEFAULT_IMPL_V2
> +#elif MINIMUM_X86_ISA_LEVEL == 3
> +# define ISA_DEFAULT_IMPL DEFAULT_IMPL_V3
> +#elif MINIMUM_X86_ISA_LEVEL == 4
> +# define ISA_DEFAULT_IMPL DEFAULT_IMPL_V4
> +#else
> +# error "Unsupport ISA Level!"
> +#endif
> +
> +#include ISA_DEFAULT_IMPL
> --
> 2.34.1
>


-- 
H.J.