diff mbox series

[v4] x86: Set minimum x86-64 level marker [BZ #27318]

Message ID CAMe9rOox=_rOpQtMiDDjCCvd=XrVqtod83jGiLjU3VRdD=KmJA@mail.gmail.com
State Superseded
Headers show
Series [v4] x86: Set minimum x86-64 level marker [BZ #27318] | expand

Commit Message

H.J. Lu March 3, 2021, 1:37 p.m. UTC
On Mon, Mar 1, 2021 at 11:09 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Mar 1, 2021 at 10:05 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu:
> >
> > >> I think we should do the following: (a) disable the build note
> > >> generation in glibc, (b) backport --list-diagnostics or something
> > >> similar.  The second part will hopefully help with analyzing failures
> > >> due to CPU support mismatches.
> > >
> > > My patch does (a).
> >
> > But not completely so, see the issue regarding post-v4 flags.
>
> It is the issue of "works on processors with the ISA level" since
> we don't check if other ISA features are required.
>
> > > But I think the ISA level meaning should be changed from "works on
> > > processors with the ISA level" to "require processors with the ISA
> > > level to work".
> >
> > Hmm.  Is this compatible with the glibc-hwcaps directory assignment
> > logic in ldconfig?
>
> Given that we don't check the extra ISA features in the binary, this
> is a logical change.

Since the full ISA set used in an ELF binary is unknown to compiler,
an x86-64 ISA level marker indicates the minimum, not maximum, ISA set
required to run such an ELF binary.  We never guarantee a library with
an x86-64 ISA level v3 marker doesn't contain other ISAs beyond x86-64
ISA level v3, like AVX VNNI.  We check the x86-64 ISA level marker for
the minimum ISA set.

Here is the updated patch.   OK for master?

Comments

Florian Weimer March 5, 2021, 7:43 p.m. UTC | #1
* H. J. Lu:

> diff --git a/sysdeps/x86/configure.ac b/sysdeps/x86/configure.ac
> index f94088f377..866dd984df 100644
> --- a/sysdeps/x86/configure.ac
> +++ b/sysdeps/x86/configure.ac
> @@ -101,11 +101,79 @@ libc_cv_include_x86_isa_level=no
>  if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -nostartfiles -nostdlib -r -o conftest conftest1.S conftest2.S); then
>    count=`LC_ALL=C $READELF -n conftest | grep NT_GNU_PROPERTY_TYPE_0 | wc -l`
>    if test "$count" = 1; then
> -    libc_cv_include_x86_isa_level=yes
> +    cat > conftest.c <<EOF
> +extern long double fmodl(long double x, long double y);
> +extern long double x, y;
> +long double
> +foo (void)
> +{
> +  return fmodl (x, y);
> +}
> +EOF
> +    ISA_LEVEL_CPPFLAGS=
> +    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -Os -ffast-math -S -o - conftest.c) | grep -q "	sahf"; then
> +      ISA_LEVEL_CPPFLAGS=-DHAS_LAHF_SAHF
> +    fi
> +    cat > conftest.c <<EOF
> +extern int x;
> +int
> +bar ()
> +{
> +  return __builtin_bswap32 (x);
> +}
> +EOF
> +    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -O2 -S -o - conftest.c) | grep -q "	movbe"; then
> +      ISA_LEVEL_CPPFLAGS="$ISA_LEVEL_CPPFLAGS -DHAS_MOVBE"
> +    fi

Please check -fverbose-asm output, I expect it to be less brittle.  Like
when new tunings result in GCC not selecting the relevant instructions.

> +DEFAULT_ISA_LEVEL=(ISA_BASELINE | ISA_V2 | ISA_V3 | ISA_V4)

I don't think it's necessary to set this as a part of a configure check.
Perhaps define macros for SAHF and MOVBE, and check them in
sysdeps/x86/isa-level.c?

But I think there's another issue: If someone builds with an option like
mno-lzcnt, it will poke a hole into the ISA level, in the sense that V2
and V4 are support, but not V3.  I think this is wrong, it should always
be an increasing progression.

Thanks,
Florian
diff mbox series

Patch

From e238dc3db8bb6950f1671c9c0b2467386e8e798d Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 2 Feb 2021 13:45:58 -0800
Subject: [PATCH v4] x86: Set minimum x86-64 level marker [BZ #27318]

Since the full ISA set used in an ELF binary is unknown to compiler,
an x86-64 ISA level marker indicates the minimum, not maximum, ISA set
required to run such an ELF binary.  We never guarantee a library with
an x86-64 ISA level v3 marker doesn't contain other ISAs beyond x86-64
ISA level v3, like AVX VNNI.  We check the x86-64 ISA level marker for
the minimum ISA set.  Since -march=sandybridge enables only some ISAs
in x86-64 ISA level v3, we should set the needed ISA marker to v2.
Otherwise, libc is compiled with -march=sandybridge will fail to run on
Sandy Bridge:

$ ./elf/ld.so ./libc.so
./libc.so: (p) CPU ISA level is lower than required: needed: 7; got: 3

1. Disable x86-64 ISA level marker for 32-bit build.
2. Set minimum, instead of maximum, x86-64 ISA level marker.

This should have no impact on the glibc-hwcaps directory assignment
logic in ldconfig and ld.so.
---
 config.h.in              |  3 ++
 sysdeps/x86/configure    | 88 +++++++++++++++++++++++++++++++++++++++-
 sysdeps/x86/configure.ac | 70 +++++++++++++++++++++++++++++++-
 sysdeps/x86/isa-level.c  | 33 +--------------
 4 files changed, 160 insertions(+), 34 deletions(-)

diff --git a/config.h.in b/config.h.in
index 06ee8ae26a..3c9035c2f2 100644
--- a/config.h.in
+++ b/config.h.in
@@ -275,4 +275,7 @@ 
 /* Define if x86 ISA level should be included in shared libraries.  */
 #undef INCLUDE_X86_ISA_LEVEL
 
+/* The default x86 ISA level.  */
+#define DEFAULT_ISA_LEVEL 0
+
 #endif
diff --git a/sysdeps/x86/configure b/sysdeps/x86/configure
index 5e32dc62b3..ffa10e2f45 100644
--- a/sysdeps/x86/configure
+++ b/sysdeps/x86/configure
@@ -134,7 +134,89 @@  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -nostartfiles -nostdlib -r -o conftest c
   test $ac_status = 0; }; }; then
   count=`LC_ALL=C $READELF -n conftest | grep NT_GNU_PROPERTY_TYPE_0 | wc -l`
   if test "$count" = 1; then
-    libc_cv_include_x86_isa_level=yes
+    cat > conftest.c <<EOF
+extern long double fmodl(long double x, long double y);
+extern long double x, y;
+long double
+foo (void)
+{
+  return fmodl (x, y);
+}
+EOF
+    ISA_LEVEL_CPPFLAGS=
+    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -Os -ffast-math -S -o - conftest.c'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; } | grep -q "	sahf"; then
+      ISA_LEVEL_CPPFLAGS=-DHAS_LAHF_SAHF
+    fi
+    cat > conftest.c <<EOF
+extern int x;
+int
+bar ()
+{
+  return __builtin_bswap32 (x);
+}
+EOF
+    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -O2 -S -o - conftest.c'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; } | grep -q "	movbe"; then
+      ISA_LEVEL_CPPFLAGS="$ISA_LEVEL_CPPFLAGS -DHAS_MOVBE"
+    fi
+    cat > conftest.c <<EOF
+#ifndef __x86_64__
+# error "x86-64 ISA level is only for x86-64"
+#endif
+
+#if defined __SSE__ && defined __SSE2__
+/* NB: ISAs in x86-64 ISA level baseline are used.  */
+# define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
+#else
+# define ISA_BASELINE	0
+#endif
+
+#if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+    && defined HAS_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 ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
+#else
+# define ISA_V2	0
+#endif
+
+#if defined __AVX__ && defined __AVX2__ && defined __F16C__ \
+    && defined __FMA__ && defined __LZCNT__ && defined HAS_MOVBE
+/* NB: Some 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 defined __AVX512F__ && defined __AVX512BW__ \
+    && defined __AVX512CD__ && defined __AVX512DQ__ \
+    && defined __AVX512VL__
+/* NB: ISAs in x86-64 ISA level v4 are used.  */
+# define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
+#else
+# define ISA_V4	0
+#endif
+
+DEFAULT_ISA_LEVEL=(ISA_BASELINE | ISA_V2 | ISA_V3 | ISA_V4)
+EOF
+    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $ISA_LEVEL_CPPFLAGS -E -o conftest.i conftest.c'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }; then
+      eval `grep DEFAULT_ISA_LEVEL= conftest.i | sed -e "s/DEFAULT_ISA_LEVEL=\(.*\)/DEFAULT_ISA_LEVEL=\"\1\"/"`
+      libc_cv_include_x86_isa_level=yes
+    fi
   fi
 fi
 rm -f conftest*
@@ -144,6 +226,10 @@  $as_echo "$libc_cv_include_x86_isa_level" >&6; }
 if test $libc_cv_include_x86_isa_level = yes; then
   $as_echo "#define INCLUDE_X86_ISA_LEVEL 1" >>confdefs.h
 
+  cat >>confdefs.h <<_ACEOF
+#define DEFAULT_ISA_LEVEL $DEFAULT_ISA_LEVEL
+_ACEOF
+
 fi
 config_vars="$config_vars
 enable-x86-isa-level = $libc_cv_include_x86_isa_level"
diff --git a/sysdeps/x86/configure.ac b/sysdeps/x86/configure.ac
index f94088f377..866dd984df 100644
--- a/sysdeps/x86/configure.ac
+++ b/sysdeps/x86/configure.ac
@@ -101,11 +101,79 @@  libc_cv_include_x86_isa_level=no
 if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -nostartfiles -nostdlib -r -o conftest conftest1.S conftest2.S); then
   count=`LC_ALL=C $READELF -n conftest | grep NT_GNU_PROPERTY_TYPE_0 | wc -l`
   if test "$count" = 1; then
-    libc_cv_include_x86_isa_level=yes
+    cat > conftest.c <<EOF
+extern long double fmodl(long double x, long double y);
+extern long double x, y;
+long double
+foo (void)
+{
+  return fmodl (x, y);
+}
+EOF
+    ISA_LEVEL_CPPFLAGS=
+    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -Os -ffast-math -S -o - conftest.c) | grep -q "	sahf"; then
+      ISA_LEVEL_CPPFLAGS=-DHAS_LAHF_SAHF
+    fi
+    cat > conftest.c <<EOF
+extern int x;
+int
+bar ()
+{
+  return __builtin_bswap32 (x);
+}
+EOF
+    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -O2 -S -o - conftest.c) | grep -q "	movbe"; then
+      ISA_LEVEL_CPPFLAGS="$ISA_LEVEL_CPPFLAGS -DHAS_MOVBE"
+    fi
+    cat > conftest.c <<EOF
+#ifndef __x86_64__
+# error "x86-64 ISA level is only for x86-64"
+#endif
+
+#if defined __SSE__ && defined __SSE2__
+/* NB: ISAs in x86-64 ISA level baseline are used.  */
+# define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
+#else
+# define ISA_BASELINE	0
+#endif
+
+#if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+    && defined HAS_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 ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
+#else
+# define ISA_V2	0
+#endif
+
+#if defined __AVX__ && defined __AVX2__ && defined __F16C__ \
+    && defined __FMA__ && defined __LZCNT__ && defined HAS_MOVBE
+/* NB: Some 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 defined __AVX512F__ && defined __AVX512BW__ \
+    && defined __AVX512CD__ && defined __AVX512DQ__ \
+    && defined __AVX512VL__
+/* NB: ISAs in x86-64 ISA level v4 are used.  */
+# define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
+#else
+# define ISA_V4	0
+#endif
+
+DEFAULT_ISA_LEVEL=(ISA_BASELINE | ISA_V2 | ISA_V3 | ISA_V4)
+EOF
+    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS $ISA_LEVEL_CPPFLAGS -E -o conftest.i conftest.c); then
+      eval `grep DEFAULT_ISA_LEVEL= conftest.i | sed -e "s/DEFAULT_ISA_LEVEL=\(.*\)/DEFAULT_ISA_LEVEL=\"\1\"/"`
+      libc_cv_include_x86_isa_level=yes
+    fi
   fi
 fi
 rm -f conftest*])
 if test $libc_cv_include_x86_isa_level = yes; then
   AC_DEFINE(INCLUDE_X86_ISA_LEVEL)
+  AC_DEFINE_UNQUOTED([DEFAULT_ISA_LEVEL], [$DEFAULT_ISA_LEVEL])
 fi
 LIBC_CONFIG_VAR([enable-x86-isa-level], [$libc_cv_include_x86_isa_level])
diff --git a/sysdeps/x86/isa-level.c b/sysdeps/x86/isa-level.c
index aaf524cb56..cb7667d9d5 100644
--- a/sysdeps/x86/isa-level.c
+++ b/sysdeps/x86/isa-level.c
@@ -29,39 +29,8 @@ 
 
 /* ELF program property for x86 ISA level.  */
 #ifdef INCLUDE_X86_ISA_LEVEL
-# if defined __x86_64__ || defined __FXSR__ || !defined _SOFT_FLOAT \
-     || defined  __MMX__ || defined __SSE__ || defined __SSE2__
-#  define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
-# else
-#  define ISA_BASELINE	0
-# endif
-
-# if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
-     || (defined __x86_64__ && defined __LAHF_SAHF__) \
-     || defined __POPCNT__ || defined __SSE3__ \
-     || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
-#  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
-# else
-#  define ISA_V2	0
-# endif
-
-# if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
-     || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__ \
-     || defined __XSAVE__
-#  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
-# else
-#  define ISA_V3	0
-# endif
-
-# if defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
-     || defined __AVX512DQ__ || defined __AVX512VL__
-#  define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
-# else
-#  define ISA_V4	0
-# endif
-
 # ifndef ISA_LEVEL
-#  define ISA_LEVEL (ISA_BASELINE | ISA_V2 | ISA_V3 | ISA_V4)
+#  define ISA_LEVEL DEFAULT_ISA_LEVEL
 # endif
 
 # if ISA_LEVEL
-- 
2.29.2