diff mbox series

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

Message ID CAMe9rOqZcK2R+eyt3TVrqGugBj7tm5eJBeYWGGiSif3hnYAxqA@mail.gmail.com
State Committed
Headers
Series [v5] x86: Set minimum x86-64 level marker [BZ #27318] |

Commit Message

H.J. Lu March 6, 2021, 3:23 p.m. UTC 
  On Fri, Mar 5, 2021 at 11:43 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * 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.

Fixed.

> > +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?

Fixed.

> 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.

Fixed.

Here is the v5 patch.   OK for master?

Thanks.

Comments

Florian Weimer March 6, 2021, 3:42 p.m. UTC | #1 
* H. J. Lu:

> Here is the v5 patch.   OK for master?

This version looks okay to me, thanks.  Are you going to apply it to the
2.33 branch as well?

Florian
H.J. Lu March 6, 2021, 3:58 p.m. UTC | #2 
On Sat, Mar 6, 2021 at 7:42 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > Here is the v5 patch.   OK for master?
>
> This version looks okay to me, thanks.  Are you going to apply it to the
> 2.33 branch as well?

Pushed into master.  I am backporting it to 2.33 branch.

Thanks.
diff mbox series

Patch

From 9ee9ba5872842d64cb5ecb95fe2bb182239201e6 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 v5] 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

Set the minimum, instead of maximum, x86-64 ISA level marker should have
no impact on the glibc-hwcaps directory assignment logic in ldconfig nor
ld.so.
---
 config.h.in              |  6 ++++++
 sysdeps/x86/configure    | 28 ++++++++++++++++++++++++++++
 sysdeps/x86/configure.ac | 16 ++++++++++++++++
 sysdeps/x86/isa-level.c  | 25 ++++++++++++++-----------
 4 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/config.h.in b/config.h.in
index 06ee8ae26a..f21bf04e47 100644
--- a/config.h.in
+++ b/config.h.in
@@ -275,4 +275,10 @@ 
 /* Define if x86 ISA level should be included in shared libraries.  */
 #undef INCLUDE_X86_ISA_LEVEL
 
+/* Define if -msahf is enabled by default on x86.  */
+#undef HAVE_X86_LAHF_SAHF
+
+/* Define if -mmovbe is enabled by default on x86.  */
+#undef HAVE_X86_MOVBE
+
 #endif
diff --git a/sysdeps/x86/configure b/sysdeps/x86/configure
index 5e32dc62b3..ead1295c38 100644
--- a/sysdeps/x86/configure
+++ b/sysdeps/x86/configure
@@ -126,6 +126,8 @@  cat > conftest2.S <<EOF
 4:
 EOF
 libc_cv_include_x86_isa_level=no
+libc_cv_have_x86_lahf_sahf=no
+libc_cv_have_x86_movbe=no
 if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -nostartfiles -nostdlib -r -o conftest conftest1.S conftest2.S'
   { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
   (eval $ac_try) 2>&5
@@ -135,6 +137,24 @@  if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -nostartfiles -nostdlib -r -o conftest c
   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
+EOF
+    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -fverbose-asm -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 "\-msahf"; then
+      libc_cv_have_x86_lahf_sahf=yes
+    fi
+    if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -fverbose-asm -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 "\-mmovbe"; then
+      libc_cv_have_x86_movbe=yes
+    fi
   fi
 fi
 rm -f conftest*
@@ -144,6 +164,14 @@  $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
 
+fi
+if test $libc_cv_have_x86_lahf_sahf = yes; then
+  $as_echo "#define HAVE_X86_LAHF_SAHF 1" >>confdefs.h
+
+fi
+if test $libc_cv_have_x86_movbe = yes; then
+  $as_echo "#define HAVE_X86_MOVBE 1" >>confdefs.h
+
 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..bca97fdc2f 100644
--- a/sysdeps/x86/configure.ac
+++ b/sysdeps/x86/configure.ac
@@ -98,14 +98,30 @@  cat > conftest2.S <<EOF
 4:
 EOF
 libc_cv_include_x86_isa_level=no
+libc_cv_have_x86_lahf_sahf=no
+libc_cv_have_x86_movbe=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
+EOF
+    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -fverbose-asm -S -o - conftest.c) | grep -q "\-msahf"; then
+      libc_cv_have_x86_lahf_sahf=yes
+    fi
+    if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -fverbose-asm -S -o - conftest.c) | grep -q "\-mmovbe"; then
+      libc_cv_have_x86_movbe=yes
+    fi
   fi
 fi
 rm -f conftest*])
 if test $libc_cv_include_x86_isa_level = yes; then
   AC_DEFINE(INCLUDE_X86_ISA_LEVEL)
 fi
+if test $libc_cv_have_x86_lahf_sahf = yes; then
+  AC_DEFINE(HAVE_X86_LAHF_SAHF)
+fi
+if test $libc_cv_have_x86_movbe = yes; then
+  AC_DEFINE(HAVE_X86_MOVBE)
+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..49ef4aa612 100644
--- a/sysdeps/x86/isa-level.c
+++ b/sysdeps/x86/isa-level.c
@@ -29,32 +29,35 @@ 
 
 /* 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__
+# if defined __SSE__ && defined __SSE2__
+/* 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 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__
+# 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__
+/* 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 __MOVBE__ \
-     || defined __XSAVE__
+# if ISA_V2 && defined __AVX__ && defined __AVX2__ && defined __F16C__ \
+     && defined __FMA__ && defined __LZCNT__ && defined HAVE_X86_MOVBE
+/* 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 defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
-     || defined __AVX512DQ__ || defined __AVX512VL__
+# if ISA_V3 && 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
-- 
2.29.2