public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318]
@ 2021-02-02 21:51 H.J. Lu
  2021-02-02 23:11 ` Joseph Myers
  2021-02-03  9:29 ` [PATCH] " Florian Weimer
  0 siblings, 2 replies; 28+ messages in thread
From: H.J. Lu @ 2021-02-02 21:51 UTC (permalink / raw)
  To: libc-alpha

Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
marker is set in libc.so.  We couldn't set the needed ISA marker to v2
since this libc won't run on all v2 machines.  Technically, the v3 marker
is correct.  But the resulting libc.so won't run on Sandy Brigde, which
is a v2 machine, even when libc is compiled with -march=sandybridge:

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

Instead, we should require full ISA support for x86-64 level marker to
detect such case:

In file included from ../sysdeps/x86/abi-note.c:28:
../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
   62 | #   error "Invalid ISAs for x86-64 ISA level v3"
      |     ^~~~~
---
 sysdeps/x86/isa-level.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/sysdeps/x86/isa-level.c b/sysdeps/x86/isa-level.c
index aaf524cb56..cbd3526406 100644
--- a/sysdeps/x86/isa-level.c
+++ b/sysdeps/x86/isa-level.c
@@ -31,6 +31,9 @@
 #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__
+#   error "Invalid ISAs for x86-64 ISA level baseline"
+#  endif
 #  define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
 # else
 #  define ISA_BASELINE	0
@@ -40,6 +43,12 @@
      || (defined __x86_64__ && defined __LAHF_SAHF__) \
      || defined __POPCNT__ || defined __SSE3__ \
      || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
+#  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__
+#   error "Invalid ISAs for x86-64 ISA level v2"
+#  endif
 #  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
 # else
 #  define ISA_V2	0
@@ -48,6 +57,10 @@
 # if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
      || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__ \
      || defined __XSAVE__
+# if !defined __AVX__ || !defined __AVX2__ || !defined __F16C__ \
+     || !defined __FMA__ || !defined __LZCNT__ || !defined __MOVBE__
+#   error "Invalid ISAs for x86-64 ISA level v3"
+#  endif
 #  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
 # else
 #  define ISA_V3	0
@@ -55,6 +68,11 @@
 
 # if defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
      || defined __AVX512DQ__ || defined __AVX512VL__
+#  if !defined __AVX512F__ || !defined __AVX512BW__ \
+      || !defined __AVX512CD__ || !defined __AVX512DQ__ \
+      || !defined __AVX512VL__
+#   error "Invalid ISAs for x86-64 ISA level v4"
+#  endif
 #  define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
 # else
 #  define ISA_V4	0
-- 
2.29.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-02 21:51 [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318] H.J. Lu
@ 2021-02-02 23:11 ` Joseph Myers
  2021-02-02 23:16   ` H.J. Lu
  2021-02-03  9:29 ` [PATCH] " Florian Weimer
  1 sibling, 1 reply; 28+ messages in thread
From: Joseph Myers @ 2021-02-02 23:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha

On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:

> Instead, we should require full ISA support for x86-64 level marker to
> detect such case:
> 
> In file included from ../sysdeps/x86/abi-note.c:28:
> ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
>    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
>       |     ^~~~~

When does this error occur (what conditions for compilation / 
configuration of glibc)?

It's definitely valid to build glibc with a compiler defaulting to any 
-march option valid for x86_64, including those between two ISA levels 
such as -march=sandybridge; that must not produce an error with default 
configure options, and must produce a glibc binary that does in fact 
execute correctly on the processor corresponding to the compiler default.

It's OK for compiling glibc with -march=sandybridge to produce a glibc 
binary that does not contain ISA level markers, or contains ISA level 
markers that permit execution on some processors that don't have all the 
instructions used in the binary.  It's not OK for it to fail to compile 
glibc, or to produce a binary that doesn't execute on Sandy Bridge.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-02 23:11 ` Joseph Myers
@ 2021-02-02 23:16   ` H.J. Lu
  2021-02-03 14:14     ` Joseph Myers
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-02 23:16 UTC (permalink / raw)
  To: Joseph Myers; +Cc: GNU C Library

On Tue, Feb 2, 2021 at 3:11 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:
>
> > Instead, we should require full ISA support for x86-64 level marker to
> > detect such case:
> >
> > In file included from ../sysdeps/x86/abi-note.c:28:
> > ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
> >    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
> >       |     ^~~~~
>
> When does this error occur (what conditions for compilation /
> configuration of glibc)?

It happens at compile time when glibc is built with "-march=sandybridge".

> It's definitely valid to build glibc with a compiler defaulting to any
> -march option valid for x86_64, including those between two ISA levels
> such as -march=sandybridge; that must not produce an error with default
> configure options, and must produce a glibc binary that does in fact
> execute correctly on the processor corresponding to the compiler default.
>
> It's OK for compiling glibc with -march=sandybridge to produce a glibc
> binary that does not contain ISA level markers, or contains ISA level
> markers that permit execution on some processors that don't have all the
> instructions used in the binary.  It's not OK for it to fail to compile
> glibc, or to produce a binary that doesn't execute on Sandy Bridge.
>

We can add a configure option, --disable-isa-level, to unset
INCLUDE_X86_ISA_LEVEL.  The resulting libc.so doesn't have a marker
and won't run on all machines.

-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-02 21:51 [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318] H.J. Lu
  2021-02-02 23:11 ` Joseph Myers
@ 2021-02-03  9:29 ` Florian Weimer
  1 sibling, 0 replies; 28+ messages in thread
From: Florian Weimer @ 2021-02-03  9:29 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha

* H. J. Lu via Libc-alpha:

> Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> marker is set in libc.so.  We couldn't set the needed ISA marker to v2
> since this libc won't run on all v2 machines.  Technically, the v3 marker
> is correct.

It's not correct due to the way the check is implemented: failing to
load -march=sandybridge binaries on Sandybridge CPUs is clearly wrong.

> But the resulting libc.so won't run on Sandy Brigde, which
> is a v2 machine, even when libc is compiled with -march=sandybridge:
>
> $ ./elf/ld.so ./libc.so
> ./libc.so: (p) CPU ISA level is lower than required: needed: 7; got: 3

They would run were it not for the check.

> Instead, we should require full ISA support for x86-64 level marker to
> detect such case:
>
> In file included from ../sysdeps/x86/abi-note.c:28:
> ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
>    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
>       |     ^~~~~

If you want checks, doing them against an incorrect ABI level that can
never fully match the host CPU is wrong.

You could do something like this:

#ifdef __SSE3___
    if (!CPU_FEATURE_USABLE_P (cpu_features, SSE3))
      _dl_fatal_printf ("Fatal glibc error: CPU does not support SSE3\n");
#endif

And repeat that for all those GCC target macros.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-02 23:16   ` H.J. Lu
@ 2021-02-03 14:14     ` Joseph Myers
  2021-02-03 15:09       ` [PATCH v2] " H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Joseph Myers @ 2021-02-03 14:14 UTC (permalink / raw)
  To: H.J. Lu; +Cc: GNU C Library

On Tue, 2 Feb 2021, H.J. Lu wrote:

> On Tue, Feb 2, 2021 at 3:11 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:
> >
> > > Instead, we should require full ISA support for x86-64 level marker to
> > > detect such case:
> > >
> > > In file included from ../sysdeps/x86/abi-note.c:28:
> > > ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
> > >    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
> > >       |     ^~~~~
> >
> > When does this error occur (what conditions for compilation /
> > configuration of glibc)?
> 
> It happens at compile time when glibc is built with "-march=sandybridge".

That's bad.  Since glibc supports execution on Sandy Bridge processors, 
compilation with -march=sandybridge should (a) work, with no special 
configure options needed and (b) produce a glibc that works on Sandy 
Bridge, with no special configure options needed.  I understand that bug 
27318 is reporting that (b) fails at present.  We need to fix (b) without 
breaking (a).

This is not specific at all to x86_64.  It applies to all architectures 
and processors supported by glibc: compiling with a compiler that defaults 
to any such processor should just work, regardless of how that processor 
relates to particular ISA levels in the glibc-hwcaps machinery.

> We can add a configure option, --disable-isa-level, to unset
> INCLUDE_X86_ISA_LEVEL.  The resulting libc.so doesn't have a marker
> and won't run on all machines.

No special configure option should be needed for (a) and (b) to hold.  
They are general principles for any processor supported by glibc, for any 
architecture.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-03 14:14     ` Joseph Myers
@ 2021-02-03 15:09       ` H.J. Lu
  2021-02-04 10:38         ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-03 15:09 UTC (permalink / raw)
  To: Joseph Myers; +Cc: GNU C Library

[-- Attachment #1: Type: text/plain, Size: 1960 bytes --]

On Wed, Feb 3, 2021 at 6:14 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 2 Feb 2021, H.J. Lu wrote:
>
> > On Tue, Feb 2, 2021 at 3:11 PM Joseph Myers <joseph@codesourcery.com> wrote:
> > >
> > > On Tue, 2 Feb 2021, H.J. Lu via Libc-alpha wrote:
> > >
> > > > Instead, we should require full ISA support for x86-64 level marker to
> > > > detect such case:
> > > >
> > > > In file included from ../sysdeps/x86/abi-note.c:28:
> > > > ../sysdeps/x86/isa-level.c:62:5: error: #error "Invalid ISAs for x86-64 ISA level v3"
> > > >    62 | #   error "Invalid ISAs for x86-64 ISA level v3"
> > > >       |     ^~~~~
> > >
> > > When does this error occur (what conditions for compilation /
> > > configuration of glibc)?
> >
> > It happens at compile time when glibc is built with "-march=sandybridge".
>
> That's bad.  Since glibc supports execution on Sandy Bridge processors,
> compilation with -march=sandybridge should (a) work, with no special
> configure options needed and (b) produce a glibc that works on Sandy
> Bridge, with no special configure options needed.  I understand that bug
> 27318 is reporting that (b) fails at present.  We need to fix (b) without
> breaking (a).
>
> This is not specific at all to x86_64.  It applies to all architectures
> and processors supported by glibc: compiling with a compiler that defaults
> to any such processor should just work, regardless of how that processor
> relates to particular ISA levels in the glibc-hwcaps machinery.
>
> > We can add a configure option, --disable-isa-level, to unset
> > INCLUDE_X86_ISA_LEVEL.  The resulting libc.so doesn't have a marker
> > and won't run on all machines.
>
> No special configure option should be needed for (a) and (b) to hold.
> They are general principles for any processor supported by glibc, for any
> architecture.

Here is the updated patch to disable x86-64 level marker for
-march=sandybridge which enables ISAs between v2 and v3.

-- 
H.J.

[-- Attachment #2: v2-0001-x86-Require-full-ISA-support-for-x86-64-level-mar.patch --]
[-- Type: text/x-patch, Size: 4652 bytes --]

From 7aa4117dabbd62d49f5b19ce36edd959d7bb260d 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 v2] x86: Require full ISA support for x86-64 level marker [BZ
 #27318]

Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
marker is set on libc.so.  We couldn't set the needed ISA marker to v2
since this libc won't run on all v2 machines.  Technically, the v3 marker
is correct.  But the resulting libc.so won't run on Sandy Brigde, which
is a v2 machine, even when libc is compiled with -march=sandybridge:

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

Instead, we require full ISA support for x86-64 level marker and disable
x86-64 level marker for -march=sandybridge which enables ISAs between v2
and v3.
---
 sysdeps/x86/configure    |  7 ++++++-
 sysdeps/x86/configure.ac |  2 +-
 sysdeps/x86/isa-level.c  | 21 ++++++++++++++++++++-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/sysdeps/x86/configure b/sysdeps/x86/configure
index 5e32dc62b3..5b20646843 100644
--- a/sysdeps/x86/configure
+++ b/sysdeps/x86/configure
@@ -133,7 +133,12 @@ if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -nostartfiles -nostdlib -r -o conftest c
   $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
   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
+  if test "$count" = 1 && { ac_try='${CC-cc} $CFLAGS $CPPFLAGS -DINCLUDE_X86_ISA_LEVEL -S -o conftest.s $srcdir/sysdeps/x86/isa-level.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
     libc_cv_include_x86_isa_level=yes
   fi
 fi
diff --git a/sysdeps/x86/configure.ac b/sysdeps/x86/configure.ac
index f94088f377..54ecd33d2c 100644
--- a/sysdeps/x86/configure.ac
+++ b/sysdeps/x86/configure.ac
@@ -100,7 +100,7 @@ EOF
 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
+  if test "$count" = 1 && AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS -DINCLUDE_X86_ISA_LEVEL -S -o conftest.s $srcdir/sysdeps/x86/isa-level.c); then
     libc_cv_include_x86_isa_level=yes
   fi
 fi
diff --git a/sysdeps/x86/isa-level.c b/sysdeps/x86/isa-level.c
index aaf524cb56..7f83449061 100644
--- a/sysdeps/x86/isa-level.c
+++ b/sysdeps/x86/isa-level.c
@@ -25,12 +25,17 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <elf.h>
+#ifdef _LIBC
+# include <elf.h>
+#endif
 
 /* 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__
+#   error "Missing ISAs for x86-64 ISA level baseline"
+#  endif
 #  define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
 # else
 #  define ISA_BASELINE	0
@@ -40,6 +45,11 @@
      || (defined __x86_64__ && defined __LAHF_SAHF__) \
      || defined __POPCNT__ || defined __SSE3__ \
      || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
+#  if !defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+     || !defined __POPCNT__ || !defined __SSE3__ \
+     || !defined __SSSE3__ || !defined __SSE4_1__ || !defined __SSE4_2__
+#   error "Missing ISAs for x86-64 ISA level v2"
+#  endif
 #  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
 # else
 #  define ISA_V2	0
@@ -48,6 +58,10 @@
 # if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
      || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__ \
      || defined __XSAVE__
+# if !defined __AVX__ || !defined __AVX2__ || !defined __F16C__ \
+     || !defined __FMA__ || !defined __LZCNT__
+#   error "Missing ISAs for x86-64 ISA level v3"
+#  endif
 #  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
 # else
 #  define ISA_V3	0
@@ -55,6 +69,11 @@
 
 # if defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
      || defined __AVX512DQ__ || defined __AVX512VL__
+#  if !defined __AVX512F__ || !defined __AVX512BW__ \
+      || !defined __AVX512CD__ || !defined __AVX512DQ__ \
+      || !defined __AVX512VL__
+#   error "Missing ISAs for x86-64 ISA level v4"
+#  endif
 #  define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
 # else
 #  define ISA_V4	0
-- 
2.29.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-03 15:09       ` [PATCH v2] " H.J. Lu
@ 2021-02-04 10:38         ` Florian Weimer
  2021-02-04 13:22           ` H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-02-04 10:38 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha; +Cc: Joseph Myers, H.J. Lu

* H. J. Lu via Libc-alpha:

> Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> marker is set on libc.so.  We couldn't set the needed ISA marker to v2
> since this libc won't run on all v2 machines.  Technically, the v3 marker
> is correct.  But the resulting libc.so won't run on Sandy Brigde, which
> is a v2 machine, even when libc is compiled with -march=sandybridge:

The v3 marker is no better than the v2 marker because it still does not
cover all Sandy Bridge features implied by -march=sandybridge.  Pretty
much all model-based -march= options have this effect.

What's the intended effect of this change?  Only require x86-64-v3 if
all x86-64-v3 features are implied by the -march= setting?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-04 10:38         ` Florian Weimer
@ 2021-02-04 13:22           ` H.J. Lu
  2021-02-04 22:55             ` Andreas K. Hüttel
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-04 13:22 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

On Thu, Feb 4, 2021 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> > marker is set on libc.so.  We couldn't set the needed ISA marker to v2
> > since this libc won't run on all v2 machines.  Technically, the v3 marker
> > is correct.  But the resulting libc.so won't run on Sandy Brigde, which
> > is a v2 machine, even when libc is compiled with -march=sandybridge:
>
> The v3 marker is no better than the v2 marker because it still does not
> cover all Sandy Bridge features implied by -march=sandybridge.  Pretty
> much all model-based -march= options have this effect.
>
> What's the intended effect of this change?  Only require x86-64-v3 if
> all x86-64-v3 features are implied by the -march= setting?
>

Yes.

-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-04 13:22           ` H.J. Lu
@ 2021-02-04 22:55             ` Andreas K. Hüttel
  2021-02-04 23:09               ` H.J. Lu
  2021-02-08 13:35               ` [PATCH v2] x86: Require full ISA support for " Nix
  0 siblings, 2 replies; 28+ messages in thread
From: Andreas K. Hüttel @ 2021-02-04 22:55 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha; +Cc: H.J. Lu via Libc-alpha, Joseph Myers, H.J. Lu

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

Am Donnerstag, 4. Februar 2021, 15:22:14 EET schrieb H.J. Lu via Libc-alpha:
> On Thu, Feb 4, 2021 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote:
> > * H. J. Lu via Libc-alpha:
> > > Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> > > marker is set on libc.so.  We couldn't set the needed ISA marker to v2
> > > since this libc won't run on all v2 machines.  Technically, the v3
> > > marker
> > > is correct.  But the resulting libc.so won't run on Sandy Brigde, which
> > 
> > > is a v2 machine, even when libc is compiled with -march=sandybridge:
> > The v3 marker is no better than the v2 marker because it still does not
> > cover all Sandy Bridge features implied by -march=sandybridge.  Pretty
> > much all model-based -march= options have this effect.
> > 
> > What's the intended effect of this change?  Only require x86-64-v3 if
> > all x86-64-v3 features are implied by the -march= setting?
> 
> Yes.

Instead of failing the build with #error, wouldn't it make more sense to 
disable ISA markers in this case?

Also, is Sandy Bridge the only case where this happens? Given the number of 
different -march arguments (plus -march=native and single feature flags), it 
seems unlikely to me that this can be mapped to a scale as simple as  1,2,3...

-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer 
(council, qa, toolchain, base-system, perl, libreoffice)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-04 22:55             ` Andreas K. Hüttel
@ 2021-02-04 23:09               ` H.J. Lu
  2021-02-07 10:27                 ` Florian Weimer
  2021-02-08 13:35               ` [PATCH v2] x86: Require full ISA support for " Nix
  1 sibling, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-04 23:09 UTC (permalink / raw)
  To: Andreas K. Hüttel
  Cc: Florian Weimer, H.J. Lu via Libc-alpha, Joseph Myers

On Thu, Feb 4, 2021 at 2:55 PM Andreas K. Hüttel <dilfridge@gentoo.org> wrote:
>
> Am Donnerstag, 4. Februar 2021, 15:22:14 EET schrieb H.J. Lu via Libc-alpha:
> > On Thu, Feb 4, 2021 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote:
> > > * H. J. Lu via Libc-alpha:
> > > > Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> > > > marker is set on libc.so.  We couldn't set the needed ISA marker to v2
> > > > since this libc won't run on all v2 machines.  Technically, the v3
> > > > marker
> > > > is correct.  But the resulting libc.so won't run on Sandy Brigde, which
> > >
> > > > is a v2 machine, even when libc is compiled with -march=sandybridge:
> > > The v3 marker is no better than the v2 marker because it still does not
> > > cover all Sandy Bridge features implied by -march=sandybridge.  Pretty
> > > much all model-based -march= options have this effect.
> > >
> > > What's the intended effect of this change?  Only require x86-64-v3 if
> > > all x86-64-v3 features are implied by the -march= setting?
> >
> > Yes.
>
> Instead of failing the build with #error, wouldn't it make more sense to
> disable ISA markers in this case?

It is exactly what this patch does.

> Also, is Sandy Bridge the only case where this happens? Given the number of
> different -march arguments (plus -march=native and single feature flags), it
> seems unlikely to me that this can be mapped to a scale as simple as  1,2,3...

It should work for all cases.

> --
> Andreas K. Hüttel
> dilfridge@gentoo.org
> Gentoo Linux developer
> (council, qa, toolchain, base-system, perl, libreoffice)



-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-04 23:09               ` H.J. Lu
@ 2021-02-07 10:27                 ` Florian Weimer
  2021-02-07 15:05                   ` H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-02-07 10:27 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Andreas K. Hüttel, H.J. Lu via Libc-alpha, Joseph Myers

* H. J. Lu:

> On Thu, Feb 4, 2021 at 2:55 PM Andreas K. Hüttel <dilfridge@gentoo.org> wrote:
>>
>> Am Donnerstag, 4. Februar 2021, 15:22:14 EET schrieb H.J. Lu via Libc-alpha:
>> > On Thu, Feb 4, 2021 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote:
>> > > * H. J. Lu via Libc-alpha:
>> > > > Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
>> > > > marker is set on libc.so.  We couldn't set the needed ISA marker to v2
>> > > > since this libc won't run on all v2 machines.  Technically, the v3
>> > > > marker
>> > > > is correct.  But the resulting libc.so won't run on Sandy Brigde, which
>> > >
>> > > > is a v2 machine, even when libc is compiled with -march=sandybridge:
>> > > The v3 marker is no better than the v2 marker because it still does not
>> > > cover all Sandy Bridge features implied by -march=sandybridge.  Pretty
>> > > much all model-based -march= options have this effect.
>> > >
>> > > What's the intended effect of this change?  Only require x86-64-v3 if
>> > > all x86-64-v3 features are implied by the -march= setting?
>> >
>> > Yes.
>>
>> Instead of failing the build with #error, wouldn't it make more sense to
>> disable ISA markers in this case?
>
> It is exactly what this patch does.

Is it possible to make this a little bit clearer in the preprocessor
conditionals?

I assume that this:

# 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 !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__
#   error "Invalid ISAs for x86-64 ISA level v2"
#  endif
#  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 !defined __AVX__ || !defined __AVX2__ || !defined __F16C__ \
     || !defined __FMA__ || !defined __LZCNT__ || !defined __MOVBE__
#   error "Invalid ISAs for x86-64 ISA level v3"
#  endif
#  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 !defined __AVX512F__ || !defined __AVX512BW__ \
      || !defined __AVX512CD__ || !defined __AVX512DQ__ \
      || !defined __AVX512VL__
#   error "Invalid ISAs for x86-64 ISA level v4"
#  endif
#  define ISA_V4        GNU_PROPERTY_X86_ISA_1_V4
# else
#  define ISA_V4        0
# endif


Could be written as:

# if TEMP_ISA_LEVEL == 1 && 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__
#  undef TEMP_ISA_LEVEL
#  define  TEMP_ISA_LEVEL 2
# endif

# if TEMP_ISA_LEVEL == 2 \
     && defined __AVX__ && defined __AVX2__ && defined __F16C__ \
     && defined __FMA__ && defined __LZCNT__ && defined __MOVBE__ \
     && defined __XSAVE__
#  undef TEMP_ISA_LEVEL
#  define  TEMP_ISA_LEVEL 3
# endif

# if TEMP_ISA_LEVEL == 3 \
     && defined __AVX512F__ && defined __AVX512BW__ && defined __AVX512CD__ \
     && defined __AVX512DQ__ && defined __AVX512VL__
#  undef TEMP_ISA_LEVEL
#  define  TEMP_ISA_LEVEL 4
# endif

So TEMP_ISA_LEVEL is moved higher and higher as long as there is
support.

I'm not sure what the point of the existing construct (|| followed by ! ||)
is.

Are ISA levels really defined for 32-bit?

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-07 10:27                 ` Florian Weimer
@ 2021-02-07 15:05                   ` H.J. Lu
  2021-02-08 15:06                     ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-07 15:05 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Andreas K. Hüttel, H.J. Lu via Libc-alpha, Joseph Myers

On Sun, Feb 7, 2021 at 2:27 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Thu, Feb 4, 2021 at 2:55 PM Andreas K. Hüttel <dilfridge@gentoo.org> wrote:
> >>
> >> Am Donnerstag, 4. Februar 2021, 15:22:14 EET schrieb H.J. Lu via Libc-alpha:
> >> > On Thu, Feb 4, 2021 at 2:38 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> > > * H. J. Lu via Libc-alpha:
> >> > > > Since -march=sandybridge enables ISAs in x86-64 ISA level v3, the v3
> >> > > > marker is set on libc.so.  We couldn't set the needed ISA marker to v2
> >> > > > since this libc won't run on all v2 machines.  Technically, the v3
> >> > > > marker
> >> > > > is correct.  But the resulting libc.so won't run on Sandy Brigde, which
> >> > >
> >> > > > is a v2 machine, even when libc is compiled with -march=sandybridge:
> >> > > The v3 marker is no better than the v2 marker because it still does not
> >> > > cover all Sandy Bridge features implied by -march=sandybridge.  Pretty
> >> > > much all model-based -march= options have this effect.
> >> > >
> >> > > What's the intended effect of this change?  Only require x86-64-v3 if
> >> > > all x86-64-v3 features are implied by the -march= setting?
> >> >
> >> > Yes.
> >>
> >> Instead of failing the build with #error, wouldn't it make more sense to
> >> disable ISA markers in this case?
> >
> > It is exactly what this patch does.
>
> Is it possible to make this a little bit clearer in the preprocessor
> conditionals?
>
> I assume that this:
>
> # 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 !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__
> #   error "Invalid ISAs for x86-64 ISA level v2"
> #  endif
> #  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 !defined __AVX__ || !defined __AVX2__ || !defined __F16C__ \
>      || !defined __FMA__ || !defined __LZCNT__ || !defined __MOVBE__
> #   error "Invalid ISAs for x86-64 ISA level v3"
> #  endif
> #  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 !defined __AVX512F__ || !defined __AVX512BW__ \
>       || !defined __AVX512CD__ || !defined __AVX512DQ__ \
>       || !defined __AVX512VL__
> #   error "Invalid ISAs for x86-64 ISA level v4"
> #  endif
> #  define ISA_V4        GNU_PROPERTY_X86_ISA_1_V4
> # else
> #  define ISA_V4        0
> # endif
>
>
> Could be written as:
>
> # if TEMP_ISA_LEVEL == 1 && 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__
> #  undef TEMP_ISA_LEVEL
> #  define  TEMP_ISA_LEVEL 2
> # endif
>
> # if TEMP_ISA_LEVEL == 2 \
>      && defined __AVX__ && defined __AVX2__ && defined __F16C__ \
>      && defined __FMA__ && defined __LZCNT__ && defined __MOVBE__ \
>      && defined __XSAVE__
> #  undef TEMP_ISA_LEVEL
> #  define  TEMP_ISA_LEVEL 3
> # endif
>
> # if TEMP_ISA_LEVEL == 3 \
>      && defined __AVX512F__ && defined __AVX512BW__ && defined __AVX512CD__ \
>      && defined __AVX512DQ__ && defined __AVX512VL__
> #  undef TEMP_ISA_LEVEL
> #  define  TEMP_ISA_LEVEL 4
> # endif
>
> So TEMP_ISA_LEVEL is moved higher and higher as long as there is
> support.
>
> I'm not sure what the point of the existing construct (|| followed by ! ||)
> is.

It is used to check if INCLUDE_X86_ISA_LEVEL should be defined in
config.h.   INCLUDE_X86_ISA_LEVEL is defined only if GCC enables
the full ISAs of a level without any ISAs of the higher ISA levels.   Since
I want to support GCC older than GCC 11,  || is used to detect any
ISAs in a level and !|| is used to check if any ISAs are missing.

> Are ISA levels really defined for 32-bit?

32-bit can just use the same x86-64 ISA level.

-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-04 22:55             ` Andreas K. Hüttel
  2021-02-04 23:09               ` H.J. Lu
@ 2021-02-08 13:35               ` Nix
  1 sibling, 0 replies; 28+ messages in thread
From: Nix @ 2021-02-08 13:35 UTC (permalink / raw)
  To: Andreas K. Hüttel via Libc-alpha
  Cc: Florian Weimer, Andreas K. Hüttel, Joseph Myers

On 4 Feb 2021, Andreas K. Hüttel via Libc-alpha outgrape:
> Also, is Sandy Bridge the only case where this happens?

Definitely not. Thank goodness I spotted this thread: I just nearly
wrecked my (32-bit) AMD Geode firewall (built with -march=geode)
installing 2.33, because it is showing up with this totally wrong
ABI-needed stamp:

Displaying notes found in: .note.gnu.property
  Owner                Data size 	Description
  GNU                  0x0000000c	NT_GNU_PROPERTY_TYPE_0
      Properties: x86 ISA needed: x86-64-v2

merely because it is built on a Broadwell CPU (Geodes are a bit too slow
to build glibc these days, and also this Geode has no compiler
installed). (My GCC is GCC 10.2 built with -march=native, but obviously
the -march=geode used at glibc compilation time usually overrides this
perfectly well and glibc works fine.)

It seems to me that this brings back the same problem we used to have
with the --enable-kernel option, where specifying a minimum kernel
version that was too high would lead to all binaries failing to execute,
only worse: if you *do* upgrade and not keep the old shared libraries
around to sln to (and don't have a statically-linked busybox shell or
something to recover), you can't recover by merely starting a suitable
kernel: you have to *physically upgrade the processor*, which is
probably impossible. (Or boot off an emergency rescue disk or something,
and recover from there). This despite the fact that a 32-bit x86 glibc
is not going to be using any of the (by definition x86-64-specific)
facilities that the ISA level is checking for, so it's not even
preventing execution for a good reason! (We don't even use SSE math on
x86-32, do we?)

Worse yet, because this is looking at the CPU ID, tests in a compilation
container are going to pass: even tests in a virtual machine will
probably pass, only to fail disastrously as soon as you install on the
real hardware.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-07 15:05                   ` H.J. Lu
@ 2021-02-08 15:06                     ` Florian Weimer
  2021-02-08 16:09                       ` H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-02-08 15:06 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha; +Cc: H.J. Lu, Joseph Myers

* H. J. Lu via Libc-alpha:

> It is used to check if INCLUDE_X86_ISA_LEVEL should be defined in
> config.h.   INCLUDE_X86_ISA_LEVEL is defined only if GCC enables
> the full ISAs of a level without any ISAs of the higher ISA levels.   Since
> I want to support GCC older than GCC 11,  || is used to detect any
> ISAs in a level and !|| is used to check if any ISAs are missing.

But isn't && the right construct anyway?  As in, if GCC was built to
include *all* feature flags for a specific level, require that level at
run time?

>> Are ISA levels really defined for 32-bit?
>
> 32-bit can just use the same x86-64 ISA level.

At least it requires figuring out the baseline level between i386 and
x86-64-v2, probably based on what GCC's -march=x86-64 option does with
-m32.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2] x86: Require full ISA support for x86-64 level marker [BZ #27318]
  2021-02-08 15:06                     ` Florian Weimer
@ 2021-02-08 16:09                       ` H.J. Lu
  2021-02-09  0:29                         ` [PATCH v3] x86: Disable " H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-08 16:09 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

On Mon, Feb 8, 2021 at 7:06 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > It is used to check if INCLUDE_X86_ISA_LEVEL should be defined in
> > config.h.   INCLUDE_X86_ISA_LEVEL is defined only if GCC enables
> > the full ISAs of a level without any ISAs of the higher ISA levels.   Since
> > I want to support GCC older than GCC 11,  || is used to detect any
> > ISAs in a level and !|| is used to check if any ISAs are missing.
>
> But isn't && the right construct anyway?  As in, if GCC was built to
> include *all* feature flags for a specific level, require that level at
> run time?

isa-level.c serves 2 purposes:

1.  Add ISA level marker (|| path) when INCLUDE_X86_ISA_LEVEL is defined.
"# error" path will never be hit.
2.  Used by sysdeps/x86/configure.ac to check if INCLUDE_X86_ISA_LEVEL
should be defined.  If any ISAs (!|| path) are missing, INCLUDE_X86_ISA_LEVEL
won't be defined.

Since GCC 10 doesn't define __LAHF_SAHF__, __MOVBE__ nor __XSAVE__,
"&&" doesn't work for GCC 10.  However, I can move ISAs check to
sysdeps/x86/configure.ac and define INCLUDE_X86_ISA_LEVEL to

#define INCLUDE_X86_ISA_LEVEL GNU_PROPERTY_X86_ISA_1_XXX

> >> Are ISA levels really defined for 32-bit?
> >
> > 32-bit can just use the same x86-64 ISA level.
>
> At least it requires figuring out the baseline level between i386 and
> x86-64-v2, probably based on what GCC's -march=x86-64 option does with
> -m32.
>

Yes, we can make the minimum i386 ISAs for glibc as i386 baseline.

-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-02-08 16:09                       ` H.J. Lu
@ 2021-02-09  0:29                         ` H.J. Lu
  2021-02-22 10:40                           ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-09  0:29 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]

On Mon, Feb 8, 2021 at 8:09 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Feb 8, 2021 at 7:06 AM Florian Weimer <fweimer@redhat.com> wrote:
> >
> > * H. J. Lu via Libc-alpha:
> >
> > > It is used to check if INCLUDE_X86_ISA_LEVEL should be defined in
> > > config.h.   INCLUDE_X86_ISA_LEVEL is defined only if GCC enables
> > > the full ISAs of a level without any ISAs of the higher ISA levels.   Since
> > > I want to support GCC older than GCC 11,  || is used to detect any
> > > ISAs in a level and !|| is used to check if any ISAs are missing.
> >
> > But isn't && the right construct anyway?  As in, if GCC was built to
> > include *all* feature flags for a specific level, require that level at
> > run time?
>
> isa-level.c serves 2 purposes:
>
> 1.  Add ISA level marker (|| path) when INCLUDE_X86_ISA_LEVEL is defined.
> "# error" path will never be hit.
> 2.  Used by sysdeps/x86/configure.ac to check if INCLUDE_X86_ISA_LEVEL
> should be defined.  If any ISAs (!|| path) are missing, INCLUDE_X86_ISA_LEVEL
> won't be defined.
>
> Since GCC 10 doesn't define __LAHF_SAHF__, __MOVBE__ nor __XSAVE__,
> "&&" doesn't work for GCC 10.  However, I can move ISAs check to
> sysdeps/x86/configure.ac and define INCLUDE_X86_ISA_LEVEL to

This patch does it.

> #define INCLUDE_X86_ISA_LEVEL GNU_PROPERTY_X86_ISA_1_XXX
>
> > >> Are ISA levels really defined for 32-bit?
> > >
> > > 32-bit can just use the same x86-64 ISA level.
> >
> > At least it requires figuring out the baseline level between i386 and
> > x86-64-v2, probably based on what GCC's -march=x86-64 option does with
> > -m32.
> >
>
> Yes, we can make the minimum i386 ISAs for glibc as i386 baseline.
>

Since i386 is legacy, this patch disables x86-64 ISA level marker for
32-bit build.
Here is the v3 patch.   Changes from v2:

1. Disable x86-64 ISA level marker for 32-bit build.
2. Disable x86-64 ISA level marker for -march=XXX if its ISA set isn't a
complete subnet of a ISA level.

-- 
H.J.

[-- Attachment #2: v3-0001-x86-Disable-x86-64-level-marker-BZ-27318.patch --]
[-- Type: text/x-patch, Size: 11973 bytes --]

From 432402b377b9ded33fd294cb2f819be139161472 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 v3] x86: Disable x86-64 level marker [BZ #27318]

Since -march=sandybridge enables some ISAs in x86-64 ISA level v3, the v3
marker is set on libc.so.  We couldn't set the needed ISA marker to v2
since this libc won't run on all v2 machines.  Technically, the v3 marker
is correct.  But the resulting libc.so won't run on Sandybrigde, which
isn't a v3 machine, even when libc is compiled with -march=sandybridge:

$ ./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. Disable x86-64 ISA level marker for -march=XXX if its ISA set isn't a
complete subnet of a ISA level.
---
 config.h.in              |   3 +
 sysdeps/x86/configure    | 119 ++++++++++++++++++++++++++++++++++++++-
 sysdeps/x86/configure.ac | 101 ++++++++++++++++++++++++++++++++-
 sysdeps/x86/isa-level.c  |  33 +----------
 4 files changed, 222 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..3812ba288e 100644
--- a/sysdeps/x86/configure
+++ b/sysdeps/x86/configure
@@ -134,7 +134,120 @@ 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=-D__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 -D__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: Some ISAs in x86-64 ISA level baseline are used.  */
+# if defined __SSE__ && defined __SSE2__
+/* NB: Enable x86-64 ISA level baseline marker only if all ISAs are
+   enabled.  */
+#  define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
+# else
+#  error "Missing ISAs for x86-64 ISA level baseline"
+# endif
+#else
+# define ISA_BASELINE	0
+#endif
+
+#if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+   || defined __LAHF_SAHF__ || defined __POPCNT__ || defined __SSE3__ \
+   || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
+/* NB: Some ISAs in x86-64 ISA level v2 are used.  */
+# if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+     && defined __LAHF_SAHF__ && defined __POPCNT__ && defined __SSE3__ \
+     && defined __SSSE3__ && defined __SSE4_1__ && defined __SSE4_2__
+/* NB: Enable x86-64 ISA level v2 marker only if all ISAs are enabled.  */
+#  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
+# else
+/* NB: We can't use the lesser x86-64 ISA level marker since some v2 ISAs
+   are used.  */
+#  error "Missing ISAs for x86-64 ISA level v2"
+# endif
+#else
+# define ISA_V2	0
+#endif
+
+#if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
+    || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__
+/* NB: Some ISAs in x86-64 ISA level v3 are used.  */
+# if defined __AVX__ && defined __AVX2__ && defined __F16C__ \
+     && defined __FMA__ && defined __LZCNT__ && defined __MOVBE__
+/* NB: Enable x86-64 ISA level v3 marker only if all ISAs are enabled.  */
+#  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
+# else
+/* NB: We can't use the lesser x86-64 ISA level marker since some v3 ISAs
+   are used.  */
+#  error "Missing ISAs for x86-64 ISA level v3"
+# endif
+#else
+# define ISA_V3	0
+#endif
+
+#if defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
+    || defined __AVX512DQ__ || defined __AVX512VL__
+/* NB: Some ISAs in x86-64 ISA level v4 are used.  */
+# if defined __AVX512F__ && defined __AVX512BW__ \
+     && defined __AVX512CD__ && defined __AVX512DQ__ \
+     && defined __AVX512VL__
+/* NB: Enable x86-64 ISA level v4 marker only if all ISAs are enabled.  */
+#  define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
+# else
+/* NB: We can't use the lesser x86-64 ISA level marker since some v4 ISAs
+   are used.  */
+#  error "Missing ISAs for x86-64 ISA level v4"
+# endif
+#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 +257,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..39d307a6d1 100644
--- a/sysdeps/x86/configure.ac
+++ b/sysdeps/x86/configure.ac
@@ -101,11 +101,110 @@ 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=-D__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 -D__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: Some ISAs in x86-64 ISA level baseline are used.  */
+# if defined __SSE__ && defined __SSE2__
+/* NB: Enable x86-64 ISA level baseline marker only if all ISAs are
+   enabled.  */
+#  define ISA_BASELINE	GNU_PROPERTY_X86_ISA_1_BASELINE
+# else
+#  error "Missing ISAs for x86-64 ISA level baseline"
+# endif
+#else
+# define ISA_BASELINE	0
+#endif
+
+#if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+   || defined __LAHF_SAHF__ || defined __POPCNT__ || defined __SSE3__ \
+   || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
+/* NB: Some ISAs in x86-64 ISA level v2 are used.  */
+# if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
+     && defined __LAHF_SAHF__ && defined __POPCNT__ && defined __SSE3__ \
+     && defined __SSSE3__ && defined __SSE4_1__ && defined __SSE4_2__
+/* NB: Enable x86-64 ISA level v2 marker only if all ISAs are enabled.  */
+#  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
+# else
+/* NB: We can't use the lesser x86-64 ISA level marker since some v2 ISAs
+   are used.  */
+#  error "Missing ISAs for x86-64 ISA level v2"
+# endif
+#else
+# define ISA_V2	0
+#endif
+
+#if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
+    || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__
+/* NB: Some ISAs in x86-64 ISA level v3 are used.  */
+# if defined __AVX__ && defined __AVX2__ && defined __F16C__ \
+     && defined __FMA__ && defined __LZCNT__ && defined __MOVBE__
+/* NB: Enable x86-64 ISA level v3 marker only if all ISAs are enabled.  */
+#  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
+# else
+/* NB: We can't use the lesser x86-64 ISA level marker since some v3 ISAs
+   are used.  */
+#  error "Missing ISAs for x86-64 ISA level v3"
+# endif
+#else
+# define ISA_V3	0
+#endif
+
+#if defined __AVX512F__ || defined __AVX512BW__ || defined __AVX512CD__ \
+    || defined __AVX512DQ__ || defined __AVX512VL__
+/* NB: Some ISAs in x86-64 ISA level v4 are used.  */
+# if defined __AVX512F__ && defined __AVX512BW__ \
+     && defined __AVX512CD__ && defined __AVX512DQ__ \
+     && defined __AVX512VL__
+/* NB: Enable x86-64 ISA level v4 marker only if all ISAs are enabled.  */
+#  define ISA_V4	GNU_PROPERTY_X86_ISA_1_V4
+# else
+/* NB: We can't use the lesser x86-64 ISA level marker since some v4 ISAs
+   are used.  */
+#  error "Missing ISAs for x86-64 ISA level v4"
+# endif
+#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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-02-09  0:29                         ` [PATCH v3] x86: Disable " H.J. Lu
@ 2021-02-22 10:40                           ` Florian Weimer
  2021-02-22 12:51                             ` H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-02-22 10:40 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha; +Cc: H.J. Lu, Joseph Myers

* H. J. Lu via Libc-alpha:

> +#if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
> +   || defined __LAHF_SAHF__ || defined __POPCNT__ || defined __SSE3__ \
> +   || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
> +/* NB: Some ISAs in x86-64 ISA level v2 are used.  */
> +# if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
> +     && defined __LAHF_SAHF__ && defined __POPCNT__ && defined __SSE3__ \
> +     && defined __SSSE3__ && defined __SSE4_1__ && defined __SSE4_2__
> +/* NB: Enable x86-64 ISA level v2 marker only if all ISAs are enabled.  */
> +#  define ISA_V2	GNU_PROPERTY_X86_ISA_1_V2
> +# else
> +/* NB: We can't use the lesser x86-64 ISA level marker since some v2 ISAs
> +   are used.  */
> +#  error "Missing ISAs for x86-64 ISA level v2"
> +# endif
> +#else
> +# define ISA_V2	0
> +#endif
> +
> +#if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
> +    || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__
> +/* NB: Some ISAs in x86-64 ISA level v3 are used.  */
> +# if defined __AVX__ && defined __AVX2__ && defined __F16C__ \
> +     && defined __FMA__ && defined __LZCNT__ && defined __MOVBE__
> +/* NB: Enable x86-64 ISA level v3 marker only if all ISAs are enabled.  */
> +#  define ISA_V3	GNU_PROPERTY_X86_ISA_1_V3
> +# else
> +/* NB: We can't use the lesser x86-64 ISA level marker since some v3 ISAs
> +   are used.  */
> +#  error "Missing ISAs for x86-64 ISA level v3"
> +# endif
> +#else
> +# define ISA_V3	0
> +#endif

I think this will produce an #error for if there is a lone -mavx in
addition to x86-64-v2 coverage in the compiler.  The first block would
define ISA_V2, but the second block produces the error.  I assume that
causes inclusion of the build note to be skipped.

Is this really helpful?  If glibc was built to run with on x86-64-v2
CPUs with some extra, wouldn't it still make sense to perform the
x86-64-v2 diagnostic upon startup?

Thanks,
Florian


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-02-22 10:40                           ` Florian Weimer
@ 2021-02-22 12:51                             ` H.J. Lu
  2021-02-22 13:51                               ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-02-22 12:51 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

On Mon, Feb 22, 2021 at 2:40 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > +#if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
> > +   || defined __LAHF_SAHF__ || defined __POPCNT__ || defined __SSE3__ \
> > +   || defined __SSSE3__ || defined __SSE4_1__ || defined __SSE4_2__
> > +/* NB: Some ISAs in x86-64 ISA level v2 are used.  */
> > +# if defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 \
> > +     && defined __LAHF_SAHF__ && defined __POPCNT__ && defined __SSE3__ \
> > +     && defined __SSSE3__ && defined __SSE4_1__ && defined __SSE4_2__
> > +/* NB: Enable x86-64 ISA level v2 marker only if all ISAs are enabled.  */
> > +#  define ISA_V2     GNU_PROPERTY_X86_ISA_1_V2
> > +# else
> > +/* NB: We can't use the lesser x86-64 ISA level marker since some v2 ISAs
> > +   are used.  */
> > +#  error "Missing ISAs for x86-64 ISA level v2"
> > +# endif
> > +#else
> > +# define ISA_V2      0
> > +#endif
> > +
> > +#if defined __AVX__ || defined __AVX2__ || defined __F16C__ \
> > +    || defined __FMA__ || defined __LZCNT__ || defined __MOVBE__
> > +/* NB: Some ISAs in x86-64 ISA level v3 are used.  */
> > +# if defined __AVX__ && defined __AVX2__ && defined __F16C__ \
> > +     && defined __FMA__ && defined __LZCNT__ && defined __MOVBE__
> > +/* NB: Enable x86-64 ISA level v3 marker only if all ISAs are enabled.  */
> > +#  define ISA_V3     GNU_PROPERTY_X86_ISA_1_V3
> > +# else
> > +/* NB: We can't use the lesser x86-64 ISA level marker since some v3 ISAs
> > +   are used.  */
> > +#  error "Missing ISAs for x86-64 ISA level v3"
> > +# endif
> > +#else
> > +# define ISA_V3      0
> > +#endif
>
> I think this will produce an #error for if there is a lone -mavx in
> addition to x86-64-v2 coverage in the compiler.  The first block would
> define ISA_V2, but the second block produces the error.  I assume that
> causes inclusion of the build note to be skipped.

Correct.

> Is this really helpful?  If glibc was built to run with on x86-64-v2
> CPUs with some extra, wouldn't it still make sense to perform the
> x86-64-v2 diagnostic upon startup?

Since libc.so now contains x86-64-v2 + extra, we can't mark it as
x86-64-v2 since it may UD on x86-64-v2 machines.  Or we can
change the meaning of the ISA level marker from "this shared library
REQUIRES ONLY x86-64-v2 to run" to "this shared library WON'T
run without x86-64-v2".


-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-02-22 12:51                             ` H.J. Lu
@ 2021-02-22 13:51                               ` Florian Weimer
  2021-03-01 16:07                                 ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-02-22 13:51 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha; +Cc: H.J. Lu, Joseph Myers

* H. J. Lu via Libc-alpha:

> On Mon, Feb 22, 2021 at 2:40 AM Florian Weimer <fweimer@redhat.com> wrote:
>> I think this will produce an #error for if there is a lone -mavx in
>> addition to x86-64-v2 coverage in the compiler.  The first block would
>> define ISA_V2, but the second block produces the error.  I assume that
>> causes inclusion of the build note to be skipped.
>
> Correct.

Okay, that explains why the checks are the way they are.  Thanks.

>> Is this really helpful?  If glibc was built to run with on x86-64-v2
>> CPUs with some extra, wouldn't it still make sense to perform the
>> x86-64-v2 diagnostic upon startup?
>
> Since libc.so now contains x86-64-v2 + extra, we can't mark it as
> x86-64-v2 since it may UD on x86-64-v2 machines.  Or we can
> change the meaning of the ISA level marker from "this shared library
> REQUIRES ONLY x86-64-v2 to run" to "this shared library WON'T
> run without x86-64-v2".

I think the loader check is worthwhile to add because otherwise people
will end up with half-working systems.  In my experience, this can lead
to lots of bug reports because people assume that if only some things
are crashing, it must be a distribution bug.  So the loader check is
quite valuable for documenting an baseline increase.

I also think that the ELF markers are valuable if they specify the exact
ABI level to which the object has been built.  But I do think we need
GCC and perhaps binutils support for generating them because otherwise,
new GCC features may silently build glibc with the wrong ABI level.  For
example, with this patch and building on GCC 10 with
-march=icelake-server, it seems like I still get a property note:

Displaying notes found in: .note.gnu.property
  Owner                Data size 	Description
  GNU                  0x00000010	NT_GNU_PROPERTY_TYPE_0	      Properties: <procesor-specific type 0xc0008002 data: 0f 00 00 00 >

But -march=icelake-server is a superset of x86-64-v4, so that's not
correct, I think.  glibc can't know a bout all these extra features, so
the generated note will always be misleading.  A note produced elsewhere
in the toolchain would not have this problem.

But we could still check for all the preprocessor macros we know that
should constitute x86-64-v2 &c, and error out in the dynamic loader if
there isn't support for them in the current process.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-02-22 13:51                               ` Florian Weimer
@ 2021-03-01 16:07                                 ` Florian Weimer
  2021-03-01 18:00                                   ` H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-03-01 16:07 UTC (permalink / raw)
  To: H.J. Lu via Libc-alpha; +Cc: H.J. Lu, Joseph Myers

* Florian Weimer:

> * H. J. Lu via Libc-alpha:
>
>> On Mon, Feb 22, 2021 at 2:40 AM Florian Weimer <fweimer@redhat.com> wrote:
>>> I think this will produce an #error for if there is a lone -mavx in
>>> addition to x86-64-v2 coverage in the compiler.  The first block would
>>> define ISA_V2, but the second block produces the error.  I assume that
>>> causes inclusion of the build note to be skipped.
>>
>> Correct.
>
> Okay, that explains why the checks are the way they are.  Thanks.
>
>>> Is this really helpful?  If glibc was built to run with on x86-64-v2
>>> CPUs with some extra, wouldn't it still make sense to perform the
>>> x86-64-v2 diagnostic upon startup?
>>
>> Since libc.so now contains x86-64-v2 + extra, we can't mark it as
>> x86-64-v2 since it may UD on x86-64-v2 machines.  Or we can
>> change the meaning of the ISA level marker from "this shared library
>> REQUIRES ONLY x86-64-v2 to run" to "this shared library WON'T
>> run without x86-64-v2".
>
> I think the loader check is worthwhile to add because otherwise people
> will end up with half-working systems.  In my experience, this can lead
> to lots of bug reports because people assume that if only some things
> are crashing, it must be a distribution bug.  So the loader check is
> quite valuable for documenting an baseline increase.
>
> I also think that the ELF markers are valuable if they specify the exact
> ABI level to which the object has been built.  But I do think we need
> GCC and perhaps binutils support for generating them because otherwise,
> new GCC features may silently build glibc with the wrong ABI level.  For
> example, with this patch and building on GCC 10 with
> -march=icelake-server, it seems like I still get a property note:
>
> Displaying notes found in: .note.gnu.property
>   Owner                Data size 	Description
>   GNU                  0x00000010	NT_GNU_PROPERTY_TYPE_0	      Properties: <procesor-specific type 0xc0008002 data: 0f 00 00 00 >
>
> But -march=icelake-server is a superset of x86-64-v4, so that's not
> correct, I think.  glibc can't know a bout all these extra features, so
> the generated note will always be misleading.  A note produced elsewhere
> in the toolchain would not have this problem.
>
> But we could still check for all the preprocessor macros we know that
> should constitute x86-64-v2 &c, and error out in the dynamic loader if
> there isn't support for them in the current process.

Any further comments here?

This bug keeps hitting more and more people.

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.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-03-01 16:07                                 ` Florian Weimer
@ 2021-03-01 18:00                                   ` H.J. Lu
  2021-03-01 18:06                                     ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-03-01 18:00 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

On Mon, Mar 1, 2021 at 8:06 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Florian Weimer:
>
> > * H. J. Lu via Libc-alpha:
> >
> >> On Mon, Feb 22, 2021 at 2:40 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>> I think this will produce an #error for if there is a lone -mavx in
> >>> addition to x86-64-v2 coverage in the compiler.  The first block would
> >>> define ISA_V2, but the second block produces the error.  I assume that
> >>> causes inclusion of the build note to be skipped.
> >>
> >> Correct.
> >
> > Okay, that explains why the checks are the way they are.  Thanks.
> >
> >>> Is this really helpful?  If glibc was built to run with on x86-64-v2
> >>> CPUs with some extra, wouldn't it still make sense to perform the
> >>> x86-64-v2 diagnostic upon startup?
> >>
> >> Since libc.so now contains x86-64-v2 + extra, we can't mark it as
> >> x86-64-v2 since it may UD on x86-64-v2 machines.  Or we can
> >> change the meaning of the ISA level marker from "this shared library
> >> REQUIRES ONLY x86-64-v2 to run" to "this shared library WON'T
> >> run without x86-64-v2".
> >
> > I think the loader check is worthwhile to add because otherwise people
> > will end up with half-working systems.  In my experience, this can lead
> > to lots of bug reports because people assume that if only some things
> > are crashing, it must be a distribution bug.  So the loader check is
> > quite valuable for documenting an baseline increase.
> >
> > I also think that the ELF markers are valuable if they specify the exact
> > ABI level to which the object has been built.  But I do think we need
> > GCC and perhaps binutils support for generating them because otherwise,
> > new GCC features may silently build glibc with the wrong ABI level.  For
> > example, with this patch and building on GCC 10 with
> > -march=icelake-server, it seems like I still get a property note:
> >
> > Displaying notes found in: .note.gnu.property
> >   Owner                Data size      Description
> >   GNU                  0x00000010     NT_GNU_PROPERTY_TYPE_0        Properties: <procesor-specific type 0xc0008002 data: 0f 00 00 00 >
> >
> > But -march=icelake-server is a superset of x86-64-v4, so that's not
> > correct, I think.  glibc can't know a bout all these extra features, so
> > the generated note will always be misleading.  A note produced elsewhere
> > in the toolchain would not have this problem.
> >
> > But we could still check for all the preprocessor macros we know that
> > should constitute x86-64-v2 &c, and error out in the dynamic loader if
> > there isn't support for them in the current process.
>
> Any further comments here?
>
> This bug keeps hitting more and more people.
>
> 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 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".

-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-03-01 18:00                                   ` H.J. Lu
@ 2021-03-01 18:06                                     ` Florian Weimer
  2021-03-01 19:09                                       ` H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-03-01 18:06 UTC (permalink / raw)
  To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

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

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

Thanks,
Florian


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3] x86: Disable x86-64 level marker [BZ #27318]
  2021-03-01 18:06                                     ` Florian Weimer
@ 2021-03-01 19:09                                       ` H.J. Lu
  2021-03-03 13:37                                         ` [PATCH v4] x86: Set minimum " H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-03-01 19:09 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

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.

-- 
H.J.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v4] x86: Set minimum x86-64 level marker [BZ #27318]
  2021-03-01 19:09                                       ` H.J. Lu
@ 2021-03-03 13:37                                         ` H.J. Lu
  2021-03-05 19:43                                           ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-03-03 13:37 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]

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?

-- 
H.J.

[-- Attachment #2: v4-0001-x86-Set-minimum-x86-64-level-marker-BZ-27318.patch --]
[-- Type: text/x-patch, Size: 9480 bytes --]

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v4] x86: Set minimum x86-64 level marker [BZ #27318]
  2021-03-03 13:37                                         ` [PATCH v4] x86: Set minimum " H.J. Lu
@ 2021-03-05 19:43                                           ` Florian Weimer
  2021-03-06 15:23                                             ` [PATCH v5] " H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-03-05 19:43 UTC (permalink / raw)
  To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

* 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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v5] x86: Set minimum x86-64 level marker [BZ #27318]
  2021-03-05 19:43                                           ` Florian Weimer
@ 2021-03-06 15:23                                             ` H.J. Lu
  2021-03-06 15:42                                               ` Florian Weimer
  0 siblings, 1 reply; 28+ messages in thread
From: H.J. Lu @ 2021-03-06 15:23 UTC (permalink / raw)
  To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 2057 bytes --]

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.

-- 
H.J.

[-- Attachment #2: v5-0001-x86-Set-minimum-x86-64-level-marker-BZ-27318.patch --]
[-- Type: text/x-patch, Size: 7033 bytes --]

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v5] x86: Set minimum x86-64 level marker [BZ #27318]
  2021-03-06 15:23                                             ` [PATCH v5] " H.J. Lu
@ 2021-03-06 15:42                                               ` Florian Weimer
  2021-03-06 15:58                                                 ` [2.33][PATCH] " H.J. Lu
  0 siblings, 1 reply; 28+ messages in thread
From: Florian Weimer @ 2021-03-06 15:42 UTC (permalink / raw)
  To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha, Joseph Myers

* 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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [2.33][PATCH] x86: Set minimum x86-64 level marker [BZ #27318]
  2021-03-06 15:42                                               ` Florian Weimer
@ 2021-03-06 15:58                                                 ` H.J. Lu
  0 siblings, 0 replies; 28+ messages in thread
From: H.J. Lu @ 2021-03-06 15:58 UTC (permalink / raw)
  To: Florian Weimer, Libc-stable Mailing List
  Cc: H.J. Lu via Libc-alpha, Joseph Myers

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

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.

-- 
H.J.

[-- Attachment #2: 0001-x86-Set-minimum-x86-64-level-marker-BZ-27318.patch --]
[-- Type: application/x-patch, Size: 7106 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-03-06 15:58 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 21:51 [PATCH] x86: Require full ISA support for x86-64 level marker [BZ #27318] H.J. Lu
2021-02-02 23:11 ` Joseph Myers
2021-02-02 23:16   ` H.J. Lu
2021-02-03 14:14     ` Joseph Myers
2021-02-03 15:09       ` [PATCH v2] " H.J. Lu
2021-02-04 10:38         ` Florian Weimer
2021-02-04 13:22           ` H.J. Lu
2021-02-04 22:55             ` Andreas K. Hüttel
2021-02-04 23:09               ` H.J. Lu
2021-02-07 10:27                 ` Florian Weimer
2021-02-07 15:05                   ` H.J. Lu
2021-02-08 15:06                     ` Florian Weimer
2021-02-08 16:09                       ` H.J. Lu
2021-02-09  0:29                         ` [PATCH v3] x86: Disable " H.J. Lu
2021-02-22 10:40                           ` Florian Weimer
2021-02-22 12:51                             ` H.J. Lu
2021-02-22 13:51                               ` Florian Weimer
2021-03-01 16:07                                 ` Florian Weimer
2021-03-01 18:00                                   ` H.J. Lu
2021-03-01 18:06                                     ` Florian Weimer
2021-03-01 19:09                                       ` H.J. Lu
2021-03-03 13:37                                         ` [PATCH v4] x86: Set minimum " H.J. Lu
2021-03-05 19:43                                           ` Florian Weimer
2021-03-06 15:23                                             ` [PATCH v5] " H.J. Lu
2021-03-06 15:42                                               ` Florian Weimer
2021-03-06 15:58                                                 ` [2.33][PATCH] " H.J. Lu
2021-02-08 13:35               ` [PATCH v2] x86: Require full ISA support for " Nix
2021-02-03  9:29 ` [PATCH] " Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).