x86: Install <sys/platform/x86.h> [BZ #26124]

Message ID CAMe9rOop09yGx5mWcQwimiCTh=BJL9=xe0mzm8wzMmQDxoc6rw@mail.gmail.com
State Superseded
Headers
Series x86: Install <sys/platform/x86.h> [BZ #26124] |

Commit Message

H.J. Lu June 18, 2020, 12:08 a.m. UTC
  On Wed, Jun 17, 2020 at 1:54 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 17 Jun 2020, H.J. Lu via Libc-alpha wrote:
>
> > Install <cpu-features.h> so that programmers can do
>
> This appears to be x86-specific.  The convention for such
> architecture-specific headers is names such as sys/platform/x86.h (with
> corresponding naming conventions for individual symbols as documented in
> maint.texi).

Done.

> This patch needs documentation in the glibc manual of the API provided by
> this header (which would also say that it's x86-specific, and go in
> platform.texi alongside other such documentation of architecture-specific
> interfaces).

Done.

> New features should also have NEWS entries.
>

Done.

Here is the updated patch to provide <sys/platform/x86.h>.

Thanks.
  

Comments

Florian Weimer June 18, 2020, 8:45 a.m. UTC | #1
* H. J. Lu via Libc-alpha:

> <sys/platform/x86.h> exports only:
>
> struct cpu_features
> {
>   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
>   unsigned int feature[FEATURE_INDEX_MAX];
>   struct cpu_features_basic basic;
> };

The struct cpu_features ABI does not appear to be stable, as I wrote on
the other thread:

  <https://sourceware.org/pipermail/libc-alpha/2020-June/115161.html>

Thanks,
Florian
  
H.J. Lu June 18, 2020, 4:14 p.m. UTC | #2
On Thu, Jun 18, 2020 at 1:45 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > <sys/platform/x86.h> exports only:
> >
> > struct cpu_features
> > {
> >   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
> >   unsigned int feature[FEATURE_INDEX_MAX];
> >   struct cpu_features_basic basic;
> > };
>
> The struct cpu_features ABI does not appear to be stable, as I wrote on
> the other thread:
>
>   <https://sourceware.org/pipermail/libc-alpha/2020-June/115161.html>
>

Here is the updated patch to use

struct cpu_features
{
  struct cpu_features_basic basic;
  unsigned int usable[USABLE_FEATURE_INDEX_MAX];
  struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
};

This should be backward compatible for both .o and .so files.
  
Florian Weimer June 22, 2020, 9:09 a.m. UTC | #3
* H. J. Lu via Libc-alpha:

> On Thu, Jun 18, 2020 at 1:45 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > <sys/platform/x86.h> exports only:
>> >
>> > struct cpu_features
>> > {
>> >   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
>> >   unsigned int feature[FEATURE_INDEX_MAX];
>> >   struct cpu_features_basic basic;
>> > };
>>
>> The struct cpu_features ABI does not appear to be stable, as I wrote on
>> the other thread:
>>
>>   <https://sourceware.org/pipermail/libc-alpha/2020-June/115161.html>
>>
>
> Here is the updated patch to use
>
> struct cpu_features
> {
>   struct cpu_features_basic basic;
>   unsigned int usable[USABLE_FEATURE_INDEX_MAX];
>   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
> };
>
> This should be backward compatible for both .o and .so files.

This is an improvement.  I still see issues with this interface, though.

Programmers will need something like this for IFUNC resolvers, but this
is not usable there if the relocation order does not match the explicit
ELF dependency order (e.g., due to symbol interposition).  This is a
generic problem with IFUNC resolvers.  Other architectures solve this by
adding arguments to IFUNC resolvers, so that the required can be
accessed without relocations.  __builtin_cpu_supports does not have this
issue.

The manual does not explain the difference between “available” and
“usable”.  I do not think we should expose both.  I don't see any
circumstances under which “available” would provide useful information.

struct cpu_features (even in its reduced form) is fairly large.  We will
never be able to reduce its size again if it becomes public ABI.

I'm not convinced that this interface conforms to the API contract for
the CPUID instruction.  Aren't the raw bits in the cpuid field subject
to interpretation based on family and model?  HAS_CPU_FEATURE does not
reflect this at all.

My preferred interface would look like this:

* The low-level function call ABI is:

  unsigned long long long int __x86_get_cpu_features (unsigned int word)
    __attribute__ ((const));

* IFUNC resolvers receive two arguments,

  unsigned long long long int *feature_words, size_t feature_words_length

* CPU_FEATURE_USABLE (NAME) would expand to something like

    ((__x86_get_cpu_features (NAME_word) & NAME_bitmask) != 0)

  The implementation of __x86_get_cpu_features simply returns 0 if the
  index is too large, which solves the extension problem.

* CPU_FEATURE_USABLE_ARRAY (WORDS, WORDS_LENGTH, NAME) would expand to this:

  (((NAME_word < WORDS_LENGTH ? WORDS[NAME_word] ? : 0) & NAME_bitmask) != 0)

  This version is expected to be used in IFUNC resolvers.  The macro
  name is just a strawman.

Only the “usable” bits are exposed, so the interface makes it clear that
the implementation performs some normalization.

Alternatively, the __builtin_cpu_supports interface could be enhanced to
cover all the features you need.

Thanks,
Florian
  

Patch

From 588eaccaf70fdecb6a3f76fe892a97c57ccf051b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 17 Jun 2020 09:12:18 -0700
Subject: [PATCH] x86: Install <sys/platform/x86.h> [BZ #26124]

Install <sys/platform/x86.h> so that programmers can do

 #if __has_include(<sys/platform/x86.h>)
 #include <sys/platform/x86.h>
 #endif
 ...

   if (HAS_CPU_FEATURE (SSE3))
 ...
   if (CPU_FEATURE_USABLE (SSE3))
 ...

<sys/platform/x86.h> exports only:

struct cpu_features
{
  struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
  unsigned int feature[FEATURE_INDEX_MAX];
  struct cpu_features_basic basic;
};

/* Get a pointer to the CPU features structure.  */
extern const struct cpu_features *__x86_get_cpu_features (void)
     __attribute__ ((const));

Since all feature checks are done through macros, programs compiled with
a newer <sys/platform/x86.h> header are compatible with the older ones as
long as the layout of struct cpu_features is identical.

Note: Although GCC has __builtin_cpu_supports, it only supports a subset
of <sys/platform/x86.h>.
---
 NEWS                                          |   2 +
 manual/platform.texi                          |  28 +++++
 sysdeps/unix/sysv/linux/i386/ld.abilist       |   1 +
 sysdeps/unix/sysv/linux/x86_64/64/ld.abilist  |   1 +
 sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist |   1 +
 sysdeps/x86/Makefile                          |   1 +
 sysdeps/x86/Versions                          |   4 +-
 sysdeps/x86/dl-get-cpu-features.c             |   4 +-
 sysdeps/x86/include/cpu-features.h            | 102 ++++++++++++++++++
 .../{cpu-features.h => sys/platform/x86.h}    |  71 ++----------
 sysdeps/x86/tst-get-cpu-features.c            |   6 +-
 11 files changed, 152 insertions(+), 69 deletions(-)
 create mode 100644 sysdeps/x86/include/cpu-features.h
 rename sysdeps/x86/{cpu-features.h => sys/platform/x86.h} (94%)

diff --git a/NEWS b/NEWS
index a660fc59a8..ae7d1ece35 100644
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,8 @@  Version 2.32
 
 Major new features:
 
+* Add <sys/platform/x86.h> to provide query macros for x86 CPU features.
+
 * Unicode 12.1.0 Support: Character encoding, character type info, and
   transliteration tables are all updated to Unicode 12.1.0, using
   generator scripts contributed by Mike FABIAN (Red Hat).
diff --git a/manual/platform.texi b/manual/platform.texi
index 504addc956..0a11f52612 100644
--- a/manual/platform.texi
+++ b/manual/platform.texi
@@ -7,6 +7,7 @@ 
 @menu
 * PowerPC::           Facilities Specific to the PowerPC Architecture
 * RISC-V::            Facilities Specific to the RISC-V Architecture
+* X86::               Facilities Specific to the X86 Architecture
 @end menu
 
 @node PowerPC
@@ -134,3 +135,30 @@  all threads in the current process.  Setting the
 ordering on only the current thread is necessary.  All other flag bits are
 reserved.
 @end deftypefun
+
+@node X86
+@appendixsec X86-specific Facilities
+
+Facilities specific to X86 that are not specific to a particular
+operating system are declared in @file{sys/platform/x86.h}.
+
+@deftypefun {const struct cpu_features *} __x86_get_cpu_features (void)
+@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
+Return a pointer to x86 CPU feature structure used by query macros for x86
+CPU features.
+@end deftypefun
+
+@deftypefn Macro int HAS_CPU_FEATURE (@var{name})
+This macro returns a nonzero value (true) if the processor feature
+@var{name} is available.
+@end deftypefn
+
+@deftypefn Macro int CPU_FEATURE_USABLE (@var{name})
+This macro returns a nonzero value (true) if the processor feature
+@var{name} is usable.
+@end deftypefn
+
+@deftypefn Macro int HAS_ARCH_FEATURE (@var{name})
+This macro returns a nonzero value (true) if the architecture feature
+@var{name} is available.
+@end deftypefn
diff --git a/sysdeps/unix/sysv/linux/i386/ld.abilist b/sysdeps/unix/sysv/linux/i386/ld.abilist
index 0478e22071..1226876689 100644
--- a/sysdeps/unix/sysv/linux/i386/ld.abilist
+++ b/sysdeps/unix/sysv/linux/i386/ld.abilist
@@ -3,3 +3,4 @@  GLIBC_2.1 __libc_stack_end D 0x4
 GLIBC_2.1 _dl_mcount F
 GLIBC_2.3 ___tls_get_addr F
 GLIBC_2.3 __tls_get_addr F
+GLIBC_2.32 __x86_get_cpu_features F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
index d3cdf7611e..886e57abd5 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/ld.abilist
@@ -2,3 +2,4 @@  GLIBC_2.2.5 __libc_stack_end D 0x8
 GLIBC_2.2.5 _dl_mcount F
 GLIBC_2.2.5 _r_debug D 0x28
 GLIBC_2.3 __tls_get_addr F
+GLIBC_2.32 __x86_get_cpu_features F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
index c70bccf782..0d2f8a2cc5 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/ld.abilist
@@ -2,3 +2,4 @@  GLIBC_2.16 __libc_stack_end D 0x4
 GLIBC_2.16 __tls_get_addr F
 GLIBC_2.16 _dl_mcount F
 GLIBC_2.16 _r_debug D 0x14
+GLIBC_2.32 __x86_get_cpu_features F
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index beab426f67..0e4d132803 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -4,6 +4,7 @@  endif
 
 ifeq ($(subdir),elf)
 sysdep-dl-routines += dl-get-cpu-features
+sysdep_headers += sys/platform/x86.h
 
 tests += tst-get-cpu-features tst-get-cpu-features-static
 tests-static += tst-get-cpu-features-static
diff --git a/sysdeps/x86/Versions b/sysdeps/x86/Versions
index e02923708e..7e3139dbb1 100644
--- a/sysdeps/x86/Versions
+++ b/sysdeps/x86/Versions
@@ -1,5 +1,5 @@ 
 ld {
-  GLIBC_PRIVATE {
-    __get_cpu_features;
+  GLIBC_2.32 {
+    __x86_get_cpu_features;
   }
 }
diff --git a/sysdeps/x86/dl-get-cpu-features.c b/sysdeps/x86/dl-get-cpu-features.c
index 9d61cd56be..fa1a1caa87 100644
--- a/sysdeps/x86/dl-get-cpu-features.c
+++ b/sysdeps/x86/dl-get-cpu-features.c
@@ -18,10 +18,10 @@ 
 
 #include <ldsodefs.h>
 
-#undef __get_cpu_features
+#undef __x86_get_cpu_features
 
 const struct cpu_features *
-__get_cpu_features (void)
+__x86_get_cpu_features (void)
 {
   return &GLRO(dl_x86_cpu_features);
 }
diff --git a/sysdeps/x86/include/cpu-features.h b/sysdeps/x86/include/cpu-features.h
new file mode 100644
index 0000000000..081483898e
--- /dev/null
+++ b/sysdeps/x86/include/cpu-features.h
@@ -0,0 +1,102 @@ 
+/* Data structure for x86 CPU features.
+   Copyright (C) 2020 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	_PRIVATE_CPU_FEATURES_H
+#define	_PRIVATE_CPU_FEATURES_H	1
+
+#ifdef _CPU_FEATURES_H
+# error this should be impossible
+#endif
+
+#ifndef _ISOMAC
+/* Get most of the contents from the public header, but we define a
+   different `struct cpu_features' type for private use.  */
+# define cpu_features		cpu_features_public
+# define __x86_get_cpu_features	__x86_get_cpu_features_public
+#endif
+
+#include <sysdeps/x86/sys/platform/x86.h>
+
+#ifndef _ISOMAC
+
+# undef	cpu_features
+# undef __x86_get_cpu_features
+# define __get_cpu_features	__x86_get_cpu_features
+
+/* XCR0 Feature flags.  */
+# define bit_XMM_state		(1u << 1)
+# define bit_YMM_state		(1u << 2)
+# define bit_Opmask_state	(1u << 5)
+# define bit_ZMM0_15_state	(1u << 6)
+# define bit_ZMM16_31_state	(1u << 7)
+
+struct cpu_features
+{
+  struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
+  unsigned int feature[FEATURE_INDEX_MAX];
+  struct cpu_features_basic basic;
+  /* The state size for XSAVEC or XSAVE.  The type must be unsigned long
+     int so that we use
+
+	sub xsave_state_size_offset(%rip) %RSP_LP
+
+     in _dl_runtime_resolve.  */
+  unsigned long int xsave_state_size;
+  /* The full state size for XSAVE when XSAVEC is disabled by
+
+     GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC_Usable
+   */
+  unsigned int xsave_state_full_size;
+  /* Data cache size for use in memory and string routines, typically
+     L1 size.  */
+  unsigned long int data_cache_size;
+  /* Shared cache size for use in memory and string routines, typically
+     L2 or L3 size.  */
+  unsigned long int shared_cache_size;
+  /* Threshold to use non temporal store.  */
+  unsigned long int non_temporal_threshold;
+};
+
+# if defined (_LIBC) && !IS_IN (nonlib)
+/* Unused for x86.  */
+#  define INIT_ARCH()
+#  define __x86_get_cpu_features()	(&GLRO(dl_x86_cpu_features))
+#  define x86_get_cpuid_registers(i) \
+       (&(GLRO(dl_x86_cpu_features).cpuid[i]))
+# endif
+
+# ifdef __x86_64__
+#  define HAS_CPUID 1
+# elif (defined __i586__ || defined __pentium__	\
+	|| defined __geode__ || defined __k6__)
+#  define HAS_CPUID 1
+#  define HAS_I586 1
+#  define HAS_I686 HAS_ARCH_FEATURE (I686)
+# elif defined __i486__
+#  define HAS_CPUID 0
+#  define HAS_I586 HAS_ARCH_FEATURE (I586)
+#  define HAS_I686 HAS_ARCH_FEATURE (I686)
+# else
+#  define HAS_CPUID 1
+#  define HAS_I586 1
+#  define HAS_I686 1
+# endif
+
+#endif /* !_ISOMAC */
+
+#endif /* include/cpu-features.h */
diff --git a/sysdeps/x86/cpu-features.h b/sysdeps/x86/sys/platform/x86.h
similarity index 94%
rename from sysdeps/x86/cpu-features.h
rename to sysdeps/x86/sys/platform/x86.h
index de20044417..2acb7fb5fa 100644
--- a/sysdeps/x86/cpu-features.h
+++ b/sysdeps/x86/sys/platform/x86.h
@@ -1,4 +1,5 @@ 
-/* This file is part of the GNU C Library.
+/* Data structure for x86 CPU features.
+   This file is part of the GNU C Library.
    Copyright (C) 2008-2020 Free Software Foundation, Inc.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -15,8 +16,8 @@ 
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#ifndef cpu_features_h
-#define cpu_features_h
+#ifndef _SYS_PLATFORM_X86_H
+#define _SYS_PLATFORM_X86_H
 
 enum
 {
@@ -72,34 +73,12 @@  struct cpu_features
   struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX];
   unsigned int feature[FEATURE_INDEX_MAX];
   struct cpu_features_basic basic;
-  /* The state size for XSAVEC or XSAVE.  The type must be unsigned long
-     int so that we use
-
-	sub xsave_state_size_offset(%rip) %RSP_LP
-
-     in _dl_runtime_resolve.  */
-  unsigned long int xsave_state_size;
-  /* The full state size for XSAVE when XSAVEC is disabled by
-
-     GLIBC_TUNABLES=glibc.cpu.hwcaps=-XSAVEC_Usable
-   */
-  unsigned int xsave_state_full_size;
-  /* Data cache size for use in memory and string routines, typically
-     L1 size.  */
-  unsigned long int data_cache_size;
-  /* Shared cache size for use in memory and string routines, typically
-     L2 or L3 size.  */
-  unsigned long int shared_cache_size;
-  /* Threshold to use non temporal store.  */
-  unsigned long int non_temporal_threshold;
 };
 
-/* Used from outside of glibc to get access to the CPU features
-   structure.  */
-extern const struct cpu_features *__get_cpu_features (void)
+/* Get a pointer to the CPU features structure.  */
+extern const struct cpu_features *__x86_get_cpu_features (void)
      __attribute__ ((const));
 
-/* Only used directly in cpu-features.c.  */
 # define CPU_FEATURES_CPU_P(ptr, name) \
   ((ptr->cpuid[index_cpu_##name].reg_##name & (bit_cpu_##name)) != 0)
 # define CPU_FEATURES_ARCH_P(ptr, name) \
@@ -107,11 +86,11 @@  extern const struct cpu_features *__get_cpu_features (void)
 
 /* HAS_CPU_FEATURE evaluates to true if CPU supports the feature.  */
 #define HAS_CPU_FEATURE(name) \
-  CPU_FEATURES_CPU_P (__get_cpu_features (), name)
+  CPU_FEATURES_CPU_P (__x86_get_cpu_features (), name)
 /* HAS_ARCH_FEATURE evaluates to true if we may use the feature at
    runtime.  */
 # define HAS_ARCH_FEATURE(name) \
-  CPU_FEATURES_ARCH_P (__get_cpu_features (), name)
+  CPU_FEATURES_ARCH_P (__x86_get_cpu_features (), name)
 /* CPU_FEATURE_USABLE evaluates to true if the feature is usable.  */
 #define CPU_FEATURE_USABLE(name)				\
   ((need_arch_feature_##name && HAS_ARCH_FEATURE (name##_Usable))	\
@@ -969,36 +948,4 @@  extern const struct cpu_features *__get_cpu_features (void)
 #define index_arch_MathVec_Prefer_No_AVX512	FEATURE_INDEX_2
 #define index_arch_Prefer_FSRM			FEATURE_INDEX_2
 
-/* XCR0 Feature flags.  */
-#define bit_XMM_state		(1u << 1)
-#define bit_YMM_state		(1u << 2)
-#define bit_Opmask_state	(1u << 5)
-#define bit_ZMM0_15_state	(1u << 6)
-#define bit_ZMM16_31_state	(1u << 7)
-
-# if defined (_LIBC) && !IS_IN (nonlib)
-/* Unused for x86.  */
-#  define INIT_ARCH()
-#  define __get_cpu_features()	(&GLRO(dl_x86_cpu_features))
-#  define x86_get_cpuid_registers(i) \
-       (&(GLRO(dl_x86_cpu_features).cpuid[i]))
-# endif
-
-#ifdef __x86_64__
-# define HAS_CPUID 1
-#elif (defined __i586__ || defined __pentium__	\
-       || defined __geode__ || defined __k6__)
-# define HAS_CPUID 1
-# define HAS_I586 1
-# define HAS_I686 HAS_ARCH_FEATURE (I686)
-#elif defined __i486__
-# define HAS_CPUID 0
-# define HAS_I586 HAS_ARCH_FEATURE (I586)
-# define HAS_I686 HAS_ARCH_FEATURE (I686)
-#else
-# define HAS_CPUID 1
-# define HAS_I586 1
-# define HAS_I686 1
-#endif
-
-#endif  /* cpu_features_h */
+#endif  /* _SYS_PLATFORM_X86_H */
diff --git a/sysdeps/x86/tst-get-cpu-features.c b/sysdeps/x86/tst-get-cpu-features.c
index 815fdb664a..010c2e2224 100644
--- a/sysdeps/x86/tst-get-cpu-features.c
+++ b/sysdeps/x86/tst-get-cpu-features.c
@@ -1,4 +1,4 @@ 
-/* Test case for x86 __get_cpu_features interface
+/* Test case for __x86_get_cpu_features interface
    Copyright (C) 2015-2020 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -18,7 +18,7 @@ 
 
 #include <stdlib.h>
 #include <stdio.h>
-#include <cpu-features.h>
+#include <sys/platform/x86.h>
 #include <support/check.h>
 
 #define CHECK_CPU_FEATURE(name)		\
@@ -45,7 +45,7 @@  static const char * const cpu_kinds[] =
 static int
 do_test (void)
 {
-  const struct cpu_features *cpu_features = __get_cpu_features ();
+  const struct cpu_features *cpu_features = __x86_get_cpu_features ();
 
   switch (cpu_features->basic.kind)
     {
-- 
2.26.2