[RFC] x86_64: Add SSE sfp-exceptions

Message ID 20200330192808.2855376-1-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • [RFC] x86_64: Add SSE sfp-exceptions
Related show

Commit Message

Sergei Trofimovich via Libc-alpha March 30, 2020, 7:28 p.m.
The exported x86_64 fenv.h functions operate on both i387 and SSE (since
they should work on both float, double, and long double) while the
internal libc_fe* set either SSE (float, double, and float128) or
i387 (long double).

The libgcc __sfp_handle_exceptions (used on float128 implementation),
however, will set either SEE or i387 exception depending of the
exception to raise.  This broke the internal assumption of float128
where only SSE operations will be used.

This patch reimplements the libgcc __sfp_handle_exceptions to use only
SSE operations and sets libgcc to use it instead of its own
implementation.

And I think we should fix libgcc in a similar manner, since checking on
config/i386/64/sfp-machine.h it already only supports SSE rounding mode
and x86_64 ABI also expectes float128 to use SSE registers [1]
(although it is not clear on how future implementation might implement
it).

Checked on x86_64-linux-gnu.

[1] https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI
---
 sysdeps/x86/fpu/sfp-exceptions.c | 58 ++++++++++++++++++++++++++++++++
 sysdeps/x86_64/fpu/Makefile      |  3 ++
 2 files changed, 61 insertions(+)
 create mode 100644 sysdeps/x86/fpu/sfp-exceptions.c

-- 
2.17.1

Comments

Sergei Trofimovich via Libc-alpha April 16, 2020, 8:35 a.m. | #1
Can somebody of the x86 maintainers have a look at this patch, please?

On 3/30/20 9:28 PM, Adhemerval Zanella via Libc-alpha wrote:
> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> they should work on both float, double, and long double) while the

> internal libc_fe* set either SSE (float, double, and float128) or

> i387 (long double).

> 

> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> however, will set either SEE or i387 exception depending of the

> exception to raise.  This broke the internal assumption of float128

> where only SSE operations will be used.

> 

> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> SSE operations and sets libgcc to use it instead of its own

> implementation.

> 

> And I think we should fix libgcc in a similar manner, since checking on

> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> and x86_64 ABI also expectes float128 to use SSE registers [1]

> (although it is not clear on how future implementation might implement

> it).

> 

> Checked on x86_64-linux-gnu.

> 

> [1] https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI

> ---

>   sysdeps/x86/fpu/sfp-exceptions.c | 58 ++++++++++++++++++++++++++++++++

>   sysdeps/x86_64/fpu/Makefile      |  3 ++

>   2 files changed, 61 insertions(+)

>   create mode 100644 sysdeps/x86/fpu/sfp-exceptions.c

> 

> diff --git a/sysdeps/x86/fpu/sfp-exceptions.c b/sysdeps/x86/fpu/sfp-exceptions.c

> new file mode 100644

> index 0000000000..2ddbfe11ba

> --- /dev/null

> +++ b/sysdeps/x86/fpu/sfp-exceptions.c

> @@ -0,0 +1,58 @@

> +/* x86_64 soft-fp exception handling for _Float128.

> +   Copyright (C) 2020 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <https://www.gnu.org/licenses/>.  */

> +

> +#include <fenv.h>

> +#include <float.h>

> +#include <math-barriers.h>

> +#include <soft-fp.h>

> +

> +void

> +__sfp_handle_exceptions (int _fex)

> +{

> +  if (_fex & FP_EX_INVALID)

> +    {

> +      float f = 0.0f;

> +      math_force_eval (f / f);

> +    }

> +  if (_fex & FP_EX_DENORM)

> +    {

> +      float f = FLT_MIN, g = 2.0f;

> +      math_force_eval (f / g);

> +    }

> +  if (_fex & FP_EX_DIVZERO)

> +    {

> +      float f = 1.0f, g = 0.0f;

> +      math_force_eval (f / g);

> +    }

> +  if (_fex & FP_EX_OVERFLOW)

> +    {

> +      float force_underflow = FLT_MAX * FLT_MAX;

> +      math_force_eval (force_underflow);

> +    }

> +  if (_fex & FP_EX_UNDERFLOW)

> +    {

> +      float force_overflow = FLT_MIN * FLT_MIN;

> +      math_force_eval (force_overflow);

> +    }

> +  if (_fex & FP_EX_INEXACT)

> +    {

> +      float f = 1.0f, g = 3.0f;

> +      math_force_eval (f / g);

> +    }

> +}

> +strong_alias (__sfp_handle_exceptions, __wrap___sfp_handle_exceptions)

> diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile

> index a4ff2723a8..5becb96fa3 100644

> --- a/sysdeps/x86_64/fpu/Makefile

> +++ b/sysdeps/x86_64/fpu/Makefile

> @@ -25,6 +25,9 @@ endif

> 

>   # Variables for libmvec tests.

>   ifeq ($(subdir),math)

> +libm-routines += sfp-exceptions

> +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> +

>   ifeq ($(build-mathvec),yes)

>   libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \

>   		 float-vlen4 float-vlen8 float-vlen8-avx2

>
Sergei Trofimovich via Libc-alpha April 16, 2020, 10:14 a.m. | #2
* Adhemerval Zanella via Libc-alpha:

> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> they should work on both float, double, and long double) while the

> internal libc_fe* set either SSE (float, double, and float128) or

> i387 (long double).

>

> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> however, will set either SEE or i387 exception depending of the

> exception to raise.  This broke the internal assumption of float128

> where only SSE operations will be used.

>

> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> SSE operations and sets libgcc to use it instead of its own

> implementation.

>

> And I think we should fix libgcc in a similar manner, since checking on

> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> and x86_64 ABI also expectes float128 to use SSE registers [1]

> (although it is not clear on how future implementation might implement

> it).


Is this another bug similar to bug 23253, fixed in commit
f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2
rounding mode for tgamma on i386 [BZ #23253]")?

Thanks,
Florian
Sergei Trofimovich via Libc-alpha April 16, 2020, 2:06 p.m. | #3
On 16/04/2020 07:14, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:

> 

>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

>> they should work on both float, double, and long double) while the

>> internal libc_fe* set either SSE (float, double, and float128) or

>> i387 (long double).

>>

>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

>> however, will set either SEE or i387 exception depending of the

>> exception to raise.  This broke the internal assumption of float128

>> where only SSE operations will be used.

>>

>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

>> SSE operations and sets libgcc to use it instead of its own

>> implementation.

>>

>> And I think we should fix libgcc in a similar manner, since checking on

>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

>> and x86_64 ABI also expectes float128 to use SSE registers [1]

>> (although it is not clear on how future implementation might implement

>> it).

> 

> Is this another bug similar to bug 23253, fixed in commit

> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

> rounding mode for tgamma on i386 [BZ #23253]")?

> 

> Thanks,

> Florian

> 


I think this is slight different because _Float128 on x86_64 is
not subject to -mfpmath switch (afaik).  The issue is libgcc
generates i387 exceptions where the expectation is it should be 
only use SSE one.
Sergei Trofimovich via Libc-alpha April 16, 2020, 2:12 p.m. | #4
On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>

>

>

> On 16/04/2020 07:14, Florian Weimer wrote:

> > * Adhemerval Zanella via Libc-alpha:

> >

> >> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> >> they should work on both float, double, and long double) while the

> >> internal libc_fe* set either SSE (float, double, and float128) or

> >> i387 (long double).

> >>

> >> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> >> however, will set either SEE or i387 exception depending of the

> >> exception to raise.  This broke the internal assumption of float128

> >> where only SSE operations will be used.

> >>

> >> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> >> SSE operations and sets libgcc to use it instead of its own

> >> implementation.

> >>

> >> And I think we should fix libgcc in a similar manner, since checking on

> >> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> >> and x86_64 ABI also expectes float128 to use SSE registers [1]

> >> (although it is not clear on how future implementation might implement

> >> it).

> >

> > Is this another bug similar to bug 23253, fixed in commit

> > f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

> > rounding mode for tgamma on i386 [BZ #23253]")?

> >

> > Thanks,

> > Florian

> >

>

> I think this is slight different because _Float128 on x86_64 is

> not subject to -mfpmath switch (afaik).  The issue is libgcc

> generates i387 exceptions where the expectation is it should be

> only use SSE one.


Should it be fixed in libgcc?

-- 
H.J.
Sergei Trofimovich via Libc-alpha April 16, 2020, 2:17 p.m. | #5
On 16/04/2020 11:12, H.J. Lu wrote:
> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

> <libc-alpha@sourceware.org> wrote:

>>

>>

>>

>> On 16/04/2020 07:14, Florian Weimer wrote:

>>> * Adhemerval Zanella via Libc-alpha:

>>>

>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

>>>> they should work on both float, double, and long double) while the

>>>> internal libc_fe* set either SSE (float, double, and float128) or

>>>> i387 (long double).

>>>>

>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

>>>> however, will set either SEE or i387 exception depending of the

>>>> exception to raise.  This broke the internal assumption of float128

>>>> where only SSE operations will be used.

>>>>

>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

>>>> SSE operations and sets libgcc to use it instead of its own

>>>> implementation.

>>>>

>>>> And I think we should fix libgcc in a similar manner, since checking on

>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

>>>> (although it is not clear on how future implementation might implement

>>>> it).

>>>

>>> Is this another bug similar to bug 23253, fixed in commit

>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

>>> rounding mode for tgamma on i386 [BZ #23253]")?

>>>

>>> Thanks,

>>> Florian

>>>

>>

>> I think this is slight different because _Float128 on x86_64 is

>> not subject to -mfpmath switch (afaik).  The issue is libgcc

>> generates i387 exceptions where the expectation is it should be

>> only use SSE one.

> 

> Should it be fixed in libgcc?


I can send a patch to libgcc, but it would require backporting to
all glibc support gcc to fix on all possible scenarios.  Should we
only go to libgcc fix or make this fix independent of the gcc
version?
Sergei Trofimovich via Libc-alpha April 16, 2020, 2:24 p.m. | #6
On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 16/04/2020 11:12, H.J. Lu wrote:

> > On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

> > <libc-alpha@sourceware.org> wrote:

> >>

> >>

> >>

> >> On 16/04/2020 07:14, Florian Weimer wrote:

> >>> * Adhemerval Zanella via Libc-alpha:

> >>>

> >>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> >>>> they should work on both float, double, and long double) while the

> >>>> internal libc_fe* set either SSE (float, double, and float128) or

> >>>> i387 (long double).

> >>>>

> >>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> >>>> however, will set either SEE or i387 exception depending of the

> >>>> exception to raise.  This broke the internal assumption of float128

> >>>> where only SSE operations will be used.

> >>>>

> >>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> >>>> SSE operations and sets libgcc to use it instead of its own

> >>>> implementation.

> >>>>

> >>>> And I think we should fix libgcc in a similar manner, since checking on

> >>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> >>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

> >>>> (although it is not clear on how future implementation might implement

> >>>> it).

> >>>

> >>> Is this another bug similar to bug 23253, fixed in commit

> >>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

> >>> rounding mode for tgamma on i386 [BZ #23253]")?

> >>>

> >>> Thanks,

> >>> Florian

> >>>

> >>

> >> I think this is slight different because _Float128 on x86_64 is

> >> not subject to -mfpmath switch (afaik).  The issue is libgcc

> >> generates i387 exceptions where the expectation is it should be

> >> only use SSE one.

> >

> > Should it be fixed in libgcc?

>

> I can send a patch to libgcc, but it would require backporting to

> all glibc support gcc to fix on all possible scenarios.  Should we

> only go to libgcc fix or make this fix independent of the gcc

> version?


I think we should do both.

Also

# Variables for libmvec tests.
 ifeq ($(subdir),math)
+libm-routines += sfp-exceptions
+LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions
+

I think you should move the "# Variables for libmvec tests." line
before "ifeq ($(build-mathvec),yes)".

Why is

LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

needed when sfp-exceptions.o is compiled into libm?

-- 
H.J.
Sergei Trofimovich via Libc-alpha April 16, 2020, 2:38 p.m. | #7
On 16/04/2020 11:24, H.J. Lu wrote:
> On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella

> <adhemerval.zanella@linaro.org> wrote:

>>

>>

>>

>> On 16/04/2020 11:12, H.J. Lu wrote:

>>> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

>>> <libc-alpha@sourceware.org> wrote:

>>>>

>>>>

>>>>

>>>> On 16/04/2020 07:14, Florian Weimer wrote:

>>>>> * Adhemerval Zanella via Libc-alpha:

>>>>>

>>>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

>>>>>> they should work on both float, double, and long double) while the

>>>>>> internal libc_fe* set either SSE (float, double, and float128) or

>>>>>> i387 (long double).

>>>>>>

>>>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

>>>>>> however, will set either SEE or i387 exception depending of the

>>>>>> exception to raise.  This broke the internal assumption of float128

>>>>>> where only SSE operations will be used.

>>>>>>

>>>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

>>>>>> SSE operations and sets libgcc to use it instead of its own

>>>>>> implementation.

>>>>>>

>>>>>> And I think we should fix libgcc in a similar manner, since checking on

>>>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

>>>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

>>>>>> (although it is not clear on how future implementation might implement

>>>>>> it).

>>>>>

>>>>> Is this another bug similar to bug 23253, fixed in commit

>>>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

>>>>> rounding mode for tgamma on i386 [BZ #23253]")?

>>>>>

>>>>> Thanks,

>>>>> Florian

>>>>>

>>>>

>>>> I think this is slight different because _Float128 on x86_64 is

>>>> not subject to -mfpmath switch (afaik).  The issue is libgcc

>>>> generates i387 exceptions where the expectation is it should be

>>>> only use SSE one.

>>>

>>> Should it be fixed in libgcc?

>>

>> I can send a patch to libgcc, but it would require backporting to

>> all glibc support gcc to fix on all possible scenarios.  Should we

>> only go to libgcc fix or make this fix independent of the gcc

>> version?

> 

> I think we should do both.

> 

> Also

> 

> # Variables for libmvec tests.

>  ifeq ($(subdir),math)

> +libm-routines += sfp-exceptions

> +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> +

> 

> I think you should move the "# Variables for libmvec tests." line

> before "ifeq ($(build-mathvec),yes)".


Ack.

> 

> Why is

> 

> LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> 

> needed when sfp-exceptions.o is compiled into libm?

> 


To force libgcc objects use the glibc provided one instead of its
own version.
Sergei Trofimovich via Libc-alpha April 16, 2020, 2:46 p.m. | #8
On Thu, Apr 16, 2020 at 7:38 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 16/04/2020 11:24, H.J. Lu wrote:

> > On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella

> > <adhemerval.zanella@linaro.org> wrote:

> >>

> >>

> >>

> >> On 16/04/2020 11:12, H.J. Lu wrote:

> >>> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

> >>> <libc-alpha@sourceware.org> wrote:

> >>>>

> >>>>

> >>>>

> >>>> On 16/04/2020 07:14, Florian Weimer wrote:

> >>>>> * Adhemerval Zanella via Libc-alpha:

> >>>>>

> >>>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> >>>>>> they should work on both float, double, and long double) while the

> >>>>>> internal libc_fe* set either SSE (float, double, and float128) or

> >>>>>> i387 (long double).

> >>>>>>

> >>>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> >>>>>> however, will set either SEE or i387 exception depending of the

> >>>>>> exception to raise.  This broke the internal assumption of float128

> >>>>>> where only SSE operations will be used.

> >>>>>>

> >>>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> >>>>>> SSE operations and sets libgcc to use it instead of its own

> >>>>>> implementation.

> >>>>>>

> >>>>>> And I think we should fix libgcc in a similar manner, since checking on

> >>>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> >>>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

> >>>>>> (although it is not clear on how future implementation might implement

> >>>>>> it).

> >>>>>

> >>>>> Is this another bug similar to bug 23253, fixed in commit

> >>>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

> >>>>> rounding mode for tgamma on i386 [BZ #23253]")?

> >>>>>

> >>>>> Thanks,

> >>>>> Florian

> >>>>>

> >>>>

> >>>> I think this is slight different because _Float128 on x86_64 is

> >>>> not subject to -mfpmath switch (afaik).  The issue is libgcc

> >>>> generates i387 exceptions where the expectation is it should be

> >>>> only use SSE one.

> >>>

> >>> Should it be fixed in libgcc?

> >>

> >> I can send a patch to libgcc, but it would require backporting to

> >> all glibc support gcc to fix on all possible scenarios.  Should we

> >> only go to libgcc fix or make this fix independent of the gcc

> >> version?

> >

> > I think we should do both.

> >

> > Also

> >

> > # Variables for libmvec tests.

> >  ifeq ($(subdir),math)

> > +libm-routines += sfp-exceptions

> > +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> > +

> >

> > I think you should move the "# Variables for libmvec tests." line

> > before "ifeq ($(build-mathvec),yes)".

>

> Ack.

>

> >

> > Why is

> >

> > LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> >

> > needed when sfp-exceptions.o is compiled into libm?

> >

>

> To force libgcc objects use the glibc provided one instead of its

> own version.


[hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc_s.so.1 | grep sfp_handle_exceptions
   254: 0000000000006bd0   143 FUNC    LOCAL  DEFAULT   14
__sfp_handle_exceptions
[hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc.a | grep sfp_handle_exceptions
    19: 0000000000000000   143 FUNC    GLOBAL HIDDEN     1
__sfp_handle_exceptions
    18: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND
__sfp_handle_exceptions

Your patch has no impact on libgcc_s.so.1.

BTW, can you include a testcase for the patch?

-- 
H.J.
Sergei Trofimovich via Libc-alpha April 17, 2020, 12:10 p.m. | #9
On 16/04/2020 11:46, H.J. Lu wrote:
> On Thu, Apr 16, 2020 at 7:38 AM Adhemerval Zanella

> <adhemerval.zanella@linaro.org> wrote:

>>

>>

>>

>> On 16/04/2020 11:24, H.J. Lu wrote:

>>> On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella

>>> <adhemerval.zanella@linaro.org> wrote:

>>>>

>>>>

>>>>

>>>> On 16/04/2020 11:12, H.J. Lu wrote:

>>>>> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

>>>>> <libc-alpha@sourceware.org> wrote:

>>>>>>

>>>>>>

>>>>>>

>>>>>> On 16/04/2020 07:14, Florian Weimer wrote:

>>>>>>> * Adhemerval Zanella via Libc-alpha:

>>>>>>>

>>>>>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

>>>>>>>> they should work on both float, double, and long double) while the

>>>>>>>> internal libc_fe* set either SSE (float, double, and float128) or

>>>>>>>> i387 (long double).

>>>>>>>>

>>>>>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

>>>>>>>> however, will set either SEE or i387 exception depending of the

>>>>>>>> exception to raise.  This broke the internal assumption of float128

>>>>>>>> where only SSE operations will be used.

>>>>>>>>

>>>>>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

>>>>>>>> SSE operations and sets libgcc to use it instead of its own

>>>>>>>> implementation.

>>>>>>>>

>>>>>>>> And I think we should fix libgcc in a similar manner, since checking on

>>>>>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

>>>>>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

>>>>>>>> (although it is not clear on how future implementation might implement

>>>>>>>> it).

>>>>>>>

>>>>>>> Is this another bug similar to bug 23253, fixed in commit

>>>>>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

>>>>>>> rounding mode for tgamma on i386 [BZ #23253]")?

>>>>>>>

>>>>>>> Thanks,

>>>>>>> Florian

>>>>>>>

>>>>>>

>>>>>> I think this is slight different because _Float128 on x86_64 is

>>>>>> not subject to -mfpmath switch (afaik).  The issue is libgcc

>>>>>> generates i387 exceptions where the expectation is it should be

>>>>>> only use SSE one.

>>>>>

>>>>> Should it be fixed in libgcc?

>>>>

>>>> I can send a patch to libgcc, but it would require backporting to

>>>> all glibc support gcc to fix on all possible scenarios.  Should we

>>>> only go to libgcc fix or make this fix independent of the gcc

>>>> version?

>>>

>>> I think we should do both.

>>>

>>> Also

>>>

>>> # Variables for libmvec tests.

>>>  ifeq ($(subdir),math)

>>> +libm-routines += sfp-exceptions

>>> +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

>>> +

>>>

>>> I think you should move the "# Variables for libmvec tests." line

>>> before "ifeq ($(build-mathvec),yes)".

>>

>> Ack.

>>

>>>

>>> Why is

>>>

>>> LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

>>>

>>> needed when sfp-exceptions.o is compiled into libm?

>>>

>>

>> To force libgcc objects use the glibc provided one instead of its

>> own version.

> 

> [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc_s.so.1 | grep sfp_handle_exceptions

>    254: 0000000000006bd0   143 FUNC    LOCAL  DEFAULT   14

> __sfp_handle_exceptions

> [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc.a | grep sfp_handle_exceptions

>     19: 0000000000000000   143 FUNC    GLOBAL HIDDEN     1

> __sfp_handle_exceptions

>     18: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND

> __sfp_handle_exceptions

> 

> Your patch has no impact on libgcc_s.so.1.

> 

> BTW, can you include a testcase for the patch?

> 


It does not affect dynamic linking with libgcc_s, but rather how to link
against libgcc.a when building libm.so itself. Instead of libgcc.a use
its own __sfp_handle_exceptions on _Float128 soft-float routines, ld will
make internal calls route to the provided wrapped implementation. 

For instance:

$ ./debugglibc.sh -- math/test-float128-exp
[...]
(gdb) b __sfp_handle_exceptions
Breakpoint 2 at 0x4076e0: __sfp_handle_exceptions. (3 locations)
(gdb) c
Continuing.
testing _Float128 (without inline functions)

Breakpoint 2, __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27
27        if (_fex & FP_EX_INVALID)
(gdb) bt
#0  __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27
#1  0x00007ffff7f2b178 in __unordtf2 (a=<optimized out>, b=1.18973149535723176508575932662800702e+4932) at /opt/cross/src/gcc/libgcc/soft-fp/unordtf2.c:45
#2  0x00007ffff7ee8191 in __ldexpf128 (exp=-16494, value=6.47517511943802511092443895822764655e-4966) at ./s_ldexp_template.c:25
#3  __ldexpf128 (value=value@entry=1, exp=exp@entry=-16494) at ./s_ldexp_template.c:21
#4  0x0000000000401f02 in ulp (value=<optimized out>, value@entry=0) at ./libm-test-support.c:597
#5  0x0000000000402061 in check_ulp () at ./libm-test-support.c:1078
#6  0x0000000000403ce6 in libm_test_init (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-support.c:1184
#7  0x0000000000401299 in main (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-driver.c:1070
#8  0x00007ffff7cedcdb in __libc_start_main (main=0x401290 <main>, argc=1, argv=0x7fffffffd300, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd2f8)
    at ../csu/libc-start.c:308
#9  0x00000000004012da in _start () at ../sysdeps/x86_64/start.S:120

And the failures from using libgcc __sfp_exceptions were found when
trying to use the internal libc optimize fenv functions [1]. I have
pushed Stefan patchset on top this patch [2] on a local branch, you
can check that by removing the --wrap build parameter the libgcc
one used and it shows the very issues noted by Stefan.

[1] https://sourceware.org/pipermail/libc-alpha/2020-March/112123.html
[2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/sfp-exceptions
Sergei Trofimovich via Libc-alpha April 17, 2020, 1:46 p.m. | #10
On Fri, Apr 17, 2020 at 5:10 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 16/04/2020 11:46, H.J. Lu wrote:

> > On Thu, Apr 16, 2020 at 7:38 AM Adhemerval Zanella

> > <adhemerval.zanella@linaro.org> wrote:

> >>

> >>

> >>

> >> On 16/04/2020 11:24, H.J. Lu wrote:

> >>> On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella

> >>> <adhemerval.zanella@linaro.org> wrote:

> >>>>

> >>>>

> >>>>

> >>>> On 16/04/2020 11:12, H.J. Lu wrote:

> >>>>> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

> >>>>> <libc-alpha@sourceware.org> wrote:

> >>>>>>

> >>>>>>

> >>>>>>

> >>>>>> On 16/04/2020 07:14, Florian Weimer wrote:

> >>>>>>> * Adhemerval Zanella via Libc-alpha:

> >>>>>>>

> >>>>>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> >>>>>>>> they should work on both float, double, and long double) while the

> >>>>>>>> internal libc_fe* set either SSE (float, double, and float128) or

> >>>>>>>> i387 (long double).

> >>>>>>>>

> >>>>>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> >>>>>>>> however, will set either SEE or i387 exception depending of the

> >>>>>>>> exception to raise.  This broke the internal assumption of float128

> >>>>>>>> where only SSE operations will be used.

> >>>>>>>>

> >>>>>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> >>>>>>>> SSE operations and sets libgcc to use it instead of its own

> >>>>>>>> implementation.

> >>>>>>>>

> >>>>>>>> And I think we should fix libgcc in a similar manner, since checking on

> >>>>>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> >>>>>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

> >>>>>>>> (although it is not clear on how future implementation might implement

> >>>>>>>> it).

> >>>>>>>

> >>>>>>> Is this another bug similar to bug 23253, fixed in commit

> >>>>>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

> >>>>>>> rounding mode for tgamma on i386 [BZ #23253]")?

> >>>>>>>

> >>>>>>> Thanks,

> >>>>>>> Florian

> >>>>>>>

> >>>>>>

> >>>>>> I think this is slight different because _Float128 on x86_64 is

> >>>>>> not subject to -mfpmath switch (afaik).  The issue is libgcc

> >>>>>> generates i387 exceptions where the expectation is it should be

> >>>>>> only use SSE one.

> >>>>>

> >>>>> Should it be fixed in libgcc?

> >>>>

> >>>> I can send a patch to libgcc, but it would require backporting to

> >>>> all glibc support gcc to fix on all possible scenarios.  Should we

> >>>> only go to libgcc fix or make this fix independent of the gcc

> >>>> version?

> >>>

> >>> I think we should do both.

> >>>

> >>> Also

> >>>

> >>> # Variables for libmvec tests.

> >>>  ifeq ($(subdir),math)

> >>> +libm-routines += sfp-exceptions

> >>> +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> >>> +

> >>>

> >>> I think you should move the "# Variables for libmvec tests." line

> >>> before "ifeq ($(build-mathvec),yes)".

> >>

> >> Ack.

> >>

> >>>

> >>> Why is

> >>>

> >>> LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> >>>

> >>> needed when sfp-exceptions.o is compiled into libm?

> >>>

> >>

> >> To force libgcc objects use the glibc provided one instead of its

> >> own version.

> >

> > [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc_s.so.1 | grep sfp_handle_exceptions

> >    254: 0000000000006bd0   143 FUNC    LOCAL  DEFAULT   14

> > __sfp_handle_exceptions

> > [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc.a | grep sfp_handle_exceptions

> >     19: 0000000000000000   143 FUNC    GLOBAL HIDDEN     1

> > __sfp_handle_exceptions

> >     18: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND

> > __sfp_handle_exceptions

> >

> > Your patch has no impact on libgcc_s.so.1.

> >

> > BTW, can you include a testcase for the patch?

> >

>

> It does not affect dynamic linking with libgcc_s, but rather how to link

> against libgcc.a when building libm.so itself. Instead of libgcc.a use

> its own __sfp_handle_exceptions on _Float128 soft-float routines, ld will

> make internal calls route to the provided wrapped implementation.

>

> For instance:

>

> $ ./debugglibc.sh -- math/test-float128-exp

> [...]

> (gdb) b __sfp_handle_exceptions

> Breakpoint 2 at 0x4076e0: __sfp_handle_exceptions. (3 locations)

> (gdb) c

> Continuing.

> testing _Float128 (without inline functions)

>

> Breakpoint 2, __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27

> 27        if (_fex & FP_EX_INVALID)

> (gdb) bt

> #0  __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27

> #1  0x00007ffff7f2b178 in __unordtf2 (a=<optimized out>, b=1.18973149535723176508575932662800702e+4932) at /opt/cross/src/gcc/libgcc/soft-fp/unordtf2.c:45

> #2  0x00007ffff7ee8191 in __ldexpf128 (exp=-16494, value=6.47517511943802511092443895822764655e-4966) at ./s_ldexp_template.c:25

> #3  __ldexpf128 (value=value@entry=1, exp=exp@entry=-16494) at ./s_ldexp_template.c:21

> #4  0x0000000000401f02 in ulp (value=<optimized out>, value@entry=0) at ./libm-test-support.c:597

> #5  0x0000000000402061 in check_ulp () at ./libm-test-support.c:1078

> #6  0x0000000000403ce6 in libm_test_init (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-support.c:1184

> #7  0x0000000000401299 in main (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-driver.c:1070

> #8  0x00007ffff7cedcdb in __libc_start_main (main=0x401290 <main>, argc=1, argv=0x7fffffffd300, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd2f8)

>     at ../csu/libc-start.c:308

> #9  0x00000000004012da in _start () at ../sysdeps/x86_64/start.S:120

>

> And the failures from using libgcc __sfp_exceptions were found when

> trying to use the internal libc optimize fenv functions [1]. I have

> pushed Stefan patchset on top this patch [2] on a local branch, you

> can check that by removing the --wrap build parameter the libgcc

> one used and it shows the very issues noted by Stefan.

>

> [1] https://sourceware.org/pipermail/libc-alpha/2020-March/112123.html

> [2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/sfp-exceptions


I applied this patch on top of azanella/sfp-exceptions branch.   There is
no regression and sysdeps/x86/fpu/sfp-exceptions.c is included in libm.so.
Did I miss something?

BTW, libgcc bug should be fixed in GCC 10 if possible.

-- 
H.J.
From ba323ff5a60679fc55112311c8d19515abdc6eeb Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 17 Apr 2020 06:43:48 -0700
Subject: [PATCH] Remove __wrap___sfp_handle_exceptions

---
 sysdeps/x86/fpu/sfp-exceptions.c | 4 ----
 sysdeps/x86_64/fpu/Makefile      | 1 -
 2 files changed, 5 deletions(-)

diff --git a/sysdeps/x86/fpu/sfp-exceptions.c b/sysdeps/x86/fpu/sfp-exceptions.c
index a60a5f9376..74ce0716e2 100644
--- a/sysdeps/x86/fpu/sfp-exceptions.c
+++ b/sysdeps/x86/fpu/sfp-exceptions.c
@@ -55,7 +55,3 @@ __sfp_handle_exceptions (int _fex)
       math_force_eval (f / g);
     }
 }
-/* The build uses a linker wrap option to force libgcc use this
-   implementation (--wrap) and it requires prepend the symbol name with
-   '__wrap_'.  */
-strong_alias (__sfp_handle_exceptions, __wrap___sfp_handle_exceptions)
diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
index aa30ba7b07..a95e3f74f4 100644
--- a/sysdeps/x86_64/fpu/Makefile
+++ b/sysdeps/x86_64/fpu/Makefile
@@ -25,7 +25,6 @@ endif
 
 ifeq ($(subdir),math)
 libm-routines += sfp-exceptions
-LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions
 
 # Variables for libmvec tests.
 ifeq ($(build-mathvec),yes)
Sergei Trofimovich via Libc-alpha April 17, 2020, 2:36 p.m. | #11
On 17/04/2020 10:46, H.J. Lu wrote:
> On Fri, Apr 17, 2020 at 5:10 AM Adhemerval Zanella

> <adhemerval.zanella@linaro.org> wrote:

>>

>>

>>

>> On 16/04/2020 11:46, H.J. Lu wrote:

>>> On Thu, Apr 16, 2020 at 7:38 AM Adhemerval Zanella

>>> <adhemerval.zanella@linaro.org> wrote:

>>>>

>>>>

>>>>

>>>> On 16/04/2020 11:24, H.J. Lu wrote:

>>>>> On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella

>>>>> <adhemerval.zanella@linaro.org> wrote:

>>>>>>

>>>>>>

>>>>>>

>>>>>> On 16/04/2020 11:12, H.J. Lu wrote:

>>>>>>> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

>>>>>>> <libc-alpha@sourceware.org> wrote:

>>>>>>>>

>>>>>>>>

>>>>>>>>

>>>>>>>> On 16/04/2020 07:14, Florian Weimer wrote:

>>>>>>>>> * Adhemerval Zanella via Libc-alpha:

>>>>>>>>>

>>>>>>>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

>>>>>>>>>> they should work on both float, double, and long double) while the

>>>>>>>>>> internal libc_fe* set either SSE (float, double, and float128) or

>>>>>>>>>> i387 (long double).

>>>>>>>>>>

>>>>>>>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

>>>>>>>>>> however, will set either SEE or i387 exception depending of the

>>>>>>>>>> exception to raise.  This broke the internal assumption of float128

>>>>>>>>>> where only SSE operations will be used.

>>>>>>>>>>

>>>>>>>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

>>>>>>>>>> SSE operations and sets libgcc to use it instead of its own

>>>>>>>>>> implementation.

>>>>>>>>>>

>>>>>>>>>> And I think we should fix libgcc in a similar manner, since checking on

>>>>>>>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

>>>>>>>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

>>>>>>>>>> (although it is not clear on how future implementation might implement

>>>>>>>>>> it).

>>>>>>>>>

>>>>>>>>> Is this another bug similar to bug 23253, fixed in commit

>>>>>>>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

>>>>>>>>> rounding mode for tgamma on i386 [BZ #23253]")?

>>>>>>>>>

>>>>>>>>> Thanks,

>>>>>>>>> Florian

>>>>>>>>>

>>>>>>>>

>>>>>>>> I think this is slight different because _Float128 on x86_64 is

>>>>>>>> not subject to -mfpmath switch (afaik).  The issue is libgcc

>>>>>>>> generates i387 exceptions where the expectation is it should be

>>>>>>>> only use SSE one.

>>>>>>>

>>>>>>> Should it be fixed in libgcc?

>>>>>>

>>>>>> I can send a patch to libgcc, but it would require backporting to

>>>>>> all glibc support gcc to fix on all possible scenarios.  Should we

>>>>>> only go to libgcc fix or make this fix independent of the gcc

>>>>>> version?

>>>>>

>>>>> I think we should do both.

>>>>>

>>>>> Also

>>>>>

>>>>> # Variables for libmvec tests.

>>>>>  ifeq ($(subdir),math)

>>>>> +libm-routines += sfp-exceptions

>>>>> +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

>>>>> +

>>>>>

>>>>> I think you should move the "# Variables for libmvec tests." line

>>>>> before "ifeq ($(build-mathvec),yes)".

>>>>

>>>> Ack.

>>>>

>>>>>

>>>>> Why is

>>>>>

>>>>> LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

>>>>>

>>>>> needed when sfp-exceptions.o is compiled into libm?

>>>>>

>>>>

>>>> To force libgcc objects use the glibc provided one instead of its

>>>> own version.

>>>

>>> [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc_s.so.1 | grep sfp_handle_exceptions

>>>    254: 0000000000006bd0   143 FUNC    LOCAL  DEFAULT   14

>>> __sfp_handle_exceptions

>>> [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc.a | grep sfp_handle_exceptions

>>>     19: 0000000000000000   143 FUNC    GLOBAL HIDDEN     1

>>> __sfp_handle_exceptions

>>>     18: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND

>>> __sfp_handle_exceptions

>>>

>>> Your patch has no impact on libgcc_s.so.1.

>>>

>>> BTW, can you include a testcase for the patch?

>>>

>>

>> It does not affect dynamic linking with libgcc_s, but rather how to link

>> against libgcc.a when building libm.so itself. Instead of libgcc.a use

>> its own __sfp_handle_exceptions on _Float128 soft-float routines, ld will

>> make internal calls route to the provided wrapped implementation.

>>

>> For instance:

>>

>> $ ./debugglibc.sh -- math/test-float128-exp

>> [...]

>> (gdb) b __sfp_handle_exceptions

>> Breakpoint 2 at 0x4076e0: __sfp_handle_exceptions. (3 locations)

>> (gdb) c

>> Continuing.

>> testing _Float128 (without inline functions)

>>

>> Breakpoint 2, __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27

>> 27        if (_fex & FP_EX_INVALID)

>> (gdb) bt

>> #0  __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27

>> #1  0x00007ffff7f2b178 in __unordtf2 (a=<optimized out>, b=1.18973149535723176508575932662800702e+4932) at /opt/cross/src/gcc/libgcc/soft-fp/unordtf2.c:45

>> #2  0x00007ffff7ee8191 in __ldexpf128 (exp=-16494, value=6.47517511943802511092443895822764655e-4966) at ./s_ldexp_template.c:25

>> #3  __ldexpf128 (value=value@entry=1, exp=exp@entry=-16494) at ./s_ldexp_template.c:21

>> #4  0x0000000000401f02 in ulp (value=<optimized out>, value@entry=0) at ./libm-test-support.c:597

>> #5  0x0000000000402061 in check_ulp () at ./libm-test-support.c:1078

>> #6  0x0000000000403ce6 in libm_test_init (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-support.c:1184

>> #7  0x0000000000401299 in main (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-driver.c:1070

>> #8  0x00007ffff7cedcdb in __libc_start_main (main=0x401290 <main>, argc=1, argv=0x7fffffffd300, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd2f8)

>>     at ../csu/libc-start.c:308

>> #9  0x00000000004012da in _start () at ../sysdeps/x86_64/start.S:120

>>

>> And the failures from using libgcc __sfp_exceptions were found when

>> trying to use the internal libc optimize fenv functions [1]. I have

>> pushed Stefan patchset on top this patch [2] on a local branch, you

>> can check that by removing the --wrap build parameter the libgcc

>> one used and it shows the very issues noted by Stefan.

>>

>> [1] https://sourceware.org/pipermail/libc-alpha/2020-March/112123.html

>> [2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/sfp-exceptions

> 

> I applied this patch on top of azanella/sfp-exceptions branch.   There is

> no regression and sysdeps/x86/fpu/sfp-exceptions.c is included in libm.so.

> Did I miss something?


Indeed the wrap trick does not seem to be required. I have removed it.
Is it ok with this change?

> 

> BTW, libgcc bug should be fixed in GCC 10 if possible.

> 


I will open a bug report and prepare a patch.
Sergei Trofimovich via Libc-alpha April 17, 2020, 2:38 p.m. | #12
On Fri, Apr 17, 2020 at 7:37 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 17/04/2020 10:46, H.J. Lu wrote:

> > On Fri, Apr 17, 2020 at 5:10 AM Adhemerval Zanella

> > <adhemerval.zanella@linaro.org> wrote:

> >>

> >>

> >>

> >> On 16/04/2020 11:46, H.J. Lu wrote:

> >>> On Thu, Apr 16, 2020 at 7:38 AM Adhemerval Zanella

> >>> <adhemerval.zanella@linaro.org> wrote:

> >>>>

> >>>>

> >>>>

> >>>> On 16/04/2020 11:24, H.J. Lu wrote:

> >>>>> On Thu, Apr 16, 2020 at 7:17 AM Adhemerval Zanella

> >>>>> <adhemerval.zanella@linaro.org> wrote:

> >>>>>>

> >>>>>>

> >>>>>>

> >>>>>> On 16/04/2020 11:12, H.J. Lu wrote:

> >>>>>>> On Thu, Apr 16, 2020 at 7:06 AM Adhemerval Zanella via Libc-alpha

> >>>>>>> <libc-alpha@sourceware.org> wrote:

> >>>>>>>>

> >>>>>>>>

> >>>>>>>>

> >>>>>>>> On 16/04/2020 07:14, Florian Weimer wrote:

> >>>>>>>>> * Adhemerval Zanella via Libc-alpha:

> >>>>>>>>>

> >>>>>>>>>> The exported x86_64 fenv.h functions operate on both i387 and SSE (since

> >>>>>>>>>> they should work on both float, double, and long double) while the

> >>>>>>>>>> internal libc_fe* set either SSE (float, double, and float128) or

> >>>>>>>>>> i387 (long double).

> >>>>>>>>>>

> >>>>>>>>>> The libgcc __sfp_handle_exceptions (used on float128 implementation),

> >>>>>>>>>> however, will set either SEE or i387 exception depending of the

> >>>>>>>>>> exception to raise.  This broke the internal assumption of float128

> >>>>>>>>>> where only SSE operations will be used.

> >>>>>>>>>>

> >>>>>>>>>> This patch reimplements the libgcc __sfp_handle_exceptions to use only

> >>>>>>>>>> SSE operations and sets libgcc to use it instead of its own

> >>>>>>>>>> implementation.

> >>>>>>>>>>

> >>>>>>>>>> And I think we should fix libgcc in a similar manner, since checking on

> >>>>>>>>>> config/i386/64/sfp-machine.h it already only supports SSE rounding mode

> >>>>>>>>>> and x86_64 ABI also expectes float128 to use SSE registers [1]

> >>>>>>>>>> (although it is not clear on how future implementation might implement

> >>>>>>>>>> it).

> >>>>>>>>>

> >>>>>>>>> Is this another bug similar to bug 23253, fixed in commit

> >>>>>>>>> f496b28e61d0342f579bf794c71b80e9c7d0b1b5 ("math: Set 387 and SSE2

> >>>>>>>>> rounding mode for tgamma on i386 [BZ #23253]")?

> >>>>>>>>>

> >>>>>>>>> Thanks,

> >>>>>>>>> Florian

> >>>>>>>>>

> >>>>>>>>

> >>>>>>>> I think this is slight different because _Float128 on x86_64 is

> >>>>>>>> not subject to -mfpmath switch (afaik).  The issue is libgcc

> >>>>>>>> generates i387 exceptions where the expectation is it should be

> >>>>>>>> only use SSE one.

> >>>>>>>

> >>>>>>> Should it be fixed in libgcc?

> >>>>>>

> >>>>>> I can send a patch to libgcc, but it would require backporting to

> >>>>>> all glibc support gcc to fix on all possible scenarios.  Should we

> >>>>>> only go to libgcc fix or make this fix independent of the gcc

> >>>>>> version?

> >>>>>

> >>>>> I think we should do both.

> >>>>>

> >>>>> Also

> >>>>>

> >>>>> # Variables for libmvec tests.

> >>>>>  ifeq ($(subdir),math)

> >>>>> +libm-routines += sfp-exceptions

> >>>>> +LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> >>>>> +

> >>>>>

> >>>>> I think you should move the "# Variables for libmvec tests." line

> >>>>> before "ifeq ($(build-mathvec),yes)".

> >>>>

> >>>> Ack.

> >>>>

> >>>>>

> >>>>> Why is

> >>>>>

> >>>>> LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions

> >>>>>

> >>>>> needed when sfp-exceptions.o is compiled into libm?

> >>>>>

> >>>>

> >>>> To force libgcc objects use the glibc provided one instead of its

> >>>> own version.

> >>>

> >>> [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc_s.so.1 | grep sfp_handle_exceptions

> >>>    254: 0000000000006bd0   143 FUNC    LOCAL  DEFAULT   14

> >>> __sfp_handle_exceptions

> >>> [hjl@gnu-cfl-2 gcc]$ readelf -sW  libgcc.a | grep sfp_handle_exceptions

> >>>     19: 0000000000000000   143 FUNC    GLOBAL HIDDEN     1

> >>> __sfp_handle_exceptions

> >>>     18: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND

> >>> __sfp_handle_exceptions

> >>>

> >>> Your patch has no impact on libgcc_s.so.1.

> >>>

> >>> BTW, can you include a testcase for the patch?

> >>>

> >>

> >> It does not affect dynamic linking with libgcc_s, but rather how to link

> >> against libgcc.a when building libm.so itself. Instead of libgcc.a use

> >> its own __sfp_handle_exceptions on _Float128 soft-float routines, ld will

> >> make internal calls route to the provided wrapped implementation.

> >>

> >> For instance:

> >>

> >> $ ./debugglibc.sh -- math/test-float128-exp

> >> [...]

> >> (gdb) b __sfp_handle_exceptions

> >> Breakpoint 2 at 0x4076e0: __sfp_handle_exceptions. (3 locations)

> >> (gdb) c

> >> Continuing.

> >> testing _Float128 (without inline functions)

> >>

> >> Breakpoint 2, __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27

> >> 27        if (_fex & FP_EX_INVALID)

> >> (gdb) bt

> >> #0  __sfp_handle_exceptions (_fex=2) at ../sysdeps/x86/fpu/sfp-exceptions.c:27

> >> #1  0x00007ffff7f2b178 in __unordtf2 (a=<optimized out>, b=1.18973149535723176508575932662800702e+4932) at /opt/cross/src/gcc/libgcc/soft-fp/unordtf2.c:45

> >> #2  0x00007ffff7ee8191 in __ldexpf128 (exp=-16494, value=6.47517511943802511092443895822764655e-4966) at ./s_ldexp_template.c:25

> >> #3  __ldexpf128 (value=value@entry=1, exp=exp@entry=-16494) at ./s_ldexp_template.c:21

> >> #4  0x0000000000401f02 in ulp (value=<optimized out>, value@entry=0) at ./libm-test-support.c:597

> >> #5  0x0000000000402061 in check_ulp () at ./libm-test-support.c:1078

> >> #6  0x0000000000403ce6 in libm_test_init (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-support.c:1184

> >> #7  0x0000000000401299 in main (argc=argc@entry=1, argv=argv@entry=0x7fffffffd300) at ./libm-test-driver.c:1070

> >> #8  0x00007ffff7cedcdb in __libc_start_main (main=0x401290 <main>, argc=1, argv=0x7fffffffd300, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd2f8)

> >>     at ../csu/libc-start.c:308

> >> #9  0x00000000004012da in _start () at ../sysdeps/x86_64/start.S:120

> >>

> >> And the failures from using libgcc __sfp_exceptions were found when

> >> trying to use the internal libc optimize fenv functions [1]. I have

> >> pushed Stefan patchset on top this patch [2] on a local branch, you

> >> can check that by removing the --wrap build parameter the libgcc

> >> one used and it shows the very issues noted by Stefan.

> >>

> >> [1] https://sourceware.org/pipermail/libc-alpha/2020-March/112123.html

> >> [2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/sfp-exceptions

> >

> > I applied this patch on top of azanella/sfp-exceptions branch.   There is

> > no regression and sysdeps/x86/fpu/sfp-exceptions.c is included in libm.so.

> > Did I miss something?

>

> Indeed the wrap trick does not seem to be required. I have removed it.

> Is it ok with this change?


Yes, please.  Thanks.

> >

> > BTW, libgcc bug should be fixed in GCC 10 if possible.

> >

>

> I will open a bug report and prepare a patch.


I appreciate it.

-- 
H.J.

Patch

diff --git a/sysdeps/x86/fpu/sfp-exceptions.c b/sysdeps/x86/fpu/sfp-exceptions.c
new file mode 100644
index 0000000000..2ddbfe11ba
--- /dev/null
+++ b/sysdeps/x86/fpu/sfp-exceptions.c
@@ -0,0 +1,58 @@ 
+/* x86_64 soft-fp exception handling for _Float128.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <fenv.h>
+#include <float.h>
+#include <math-barriers.h>
+#include <soft-fp.h>
+
+void
+__sfp_handle_exceptions (int _fex)
+{
+  if (_fex & FP_EX_INVALID)
+    {
+      float f = 0.0f;
+      math_force_eval (f / f);
+    }
+  if (_fex & FP_EX_DENORM)
+    {
+      float f = FLT_MIN, g = 2.0f;
+      math_force_eval (f / g);
+    }
+  if (_fex & FP_EX_DIVZERO)
+    {
+      float f = 1.0f, g = 0.0f;
+      math_force_eval (f / g);
+    }
+  if (_fex & FP_EX_OVERFLOW)
+    {
+      float force_underflow = FLT_MAX * FLT_MAX;
+      math_force_eval (force_underflow);
+    }
+  if (_fex & FP_EX_UNDERFLOW)
+    {
+      float force_overflow = FLT_MIN * FLT_MIN;
+      math_force_eval (force_overflow);
+    }
+  if (_fex & FP_EX_INEXACT)
+    {
+      float f = 1.0f, g = 3.0f;
+      math_force_eval (f / g);
+    }
+}
+strong_alias (__sfp_handle_exceptions, __wrap___sfp_handle_exceptions)
diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
index a4ff2723a8..5becb96fa3 100644
--- a/sysdeps/x86_64/fpu/Makefile
+++ b/sysdeps/x86_64/fpu/Makefile
@@ -25,6 +25,9 @@  endif
 
 # Variables for libmvec tests.
 ifeq ($(subdir),math)
+libm-routines += sfp-exceptions
+LDFLAGS-m.so += -Wl,--wrap=__sfp_handle_exceptions
+
 ifeq ($(build-mathvec),yes)
 libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \
 		 float-vlen4 float-vlen8 float-vlen8-avx2