public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v3] Fix for powerpc64 long double complex divide failure
@ 2021-08-12 16:03 Patrick McGehearty
  2021-08-12 16:19 ` Joseph Myers
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Patrick McGehearty @ 2021-08-12 16:03 UTC (permalink / raw)
  To: gcc-patches; +Cc: segher

This patch resolves the failure of powerpc64 long double complex divide
in native ibm long double format after the patch "Practical improvement
to libgcc complex divide".

The new code uses the following macros which are intended to be mapped
to appropriate values according to the underlying hardware representation.
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101104

RBIG     a value near the maximum representation
RMIN     a value near the minimum representation
         (but not in the subnormal range)
RMIN2    a value moderately less than 1
RMINSCAL the inverse of RMIN2
RMAX2    RBIG * RMIN2  - a value to limit scaling to not overflow

When "long double" values were not using the IEEE 128-bit format but
the traditional IBM 128-bit, the previous code used the LDBL values
which caused overflow for RMINSCAL. The new code uses the DBL values.

RBIG  LDBL_MAX = 0x1.fffffffffffff800p+1022
      DBL_MAX  = 0x1.fffffffffffff000p+1022

RMIN  LDBL_MIN = 0x1.0000000000000000p-969
RMIN  DBL_MIN  = 0x1.0000000000000000p-1022

RMIN2 LDBL_EPSILON = 0x0.0000000000001000p-1022 = 0x1.0p-1074
RMIN2 DBL_EPSILON  = 0x1.0000000000000000p-52

RMINSCAL 1/LDBL_EPSILON = inf (1.0p+1074 does not fit in IBM 128-bit).
         1/DBL_EPSILON  = 0x1.0000000000000000p+52

RMAX2 = RBIG * RMIN2 = 0x1.fffffffffffff800p-52
        RBIG * RMIN2 = 0x1.fffffffffffff000p+970

The MAX and MIN values have only modest changes since the maximum and
minimum values are about the same as for double precision.  The
EPSILON field is considerably different. Due to how very small values
can be represented in the lower 64 bits of the IBM 128-bit floating
point, EPSILON is extremely small, so far beyond the desired value
that inversion of the value overflows and even without the overflow,
the RMAX2 is so small as to eliminate most usage of the test.

Instead of just replacing the use of KF_EPSILON with DF_EPSILON, we
replace all uses of KF_* with DF_*. Since the exponent fields are
essentially the same, we gain the positive benefits from the new
formula while avoiding all under/overflow issues in the #defines.

The change has been tested on gcc135.fsffrance.org and gains the
expected improvements in accuracy for long double complex divide.

libgcc/
	PR target/101104
	* config/rs6000/_divkc3.c (RBIG, RMIN, RMIN2, RMINSCAL, RMAX2):
	Use more correct values for native IBM 128-bit.
---
 libgcc/config/rs6000/_divkc3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libgcc/config/rs6000/_divkc3.c b/libgcc/config/rs6000/_divkc3.c
index a1d29d2..2b229c8 100644
--- a/libgcc/config/rs6000/_divkc3.c
+++ b/libgcc/config/rs6000/_divkc3.c
@@ -38,10 +38,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #endif
 
 #ifndef __LONG_DOUBLE_IEEE128__
-#define RBIG   (__LIBGCC_KF_MAX__ / 2)
-#define RMIN   (__LIBGCC_KF_MIN__)
-#define RMIN2  (__LIBGCC_KF_EPSILON__)
-#define RMINSCAL (1 / __LIBGCC_KF_EPSILON__)
+#define RBIG   (__LIBGCC_DF_MAX__ / 2)
+#define RMIN   (__LIBGCC_DF_MIN__)
+#define RMIN2  (__LIBGCC_DF_EPSILON__)
+#define RMINSCAL (1 / __LIBGCC_DF_EPSILON__)
 #define RMAX2  (RBIG * RMIN2)
 #else
 #define RBIG   (__LIBGCC_TF_MAX__ / 2)
-- 
1.8.3.1


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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:03 [PATCH v3] Fix for powerpc64 long double complex divide failure Patrick McGehearty
@ 2021-08-12 16:19 ` Joseph Myers
  2021-08-12 16:40   ` Patrick McGehearty
  2021-08-12 20:55 ` Segher Boessenkool
  2021-08-12 22:36 ` Andreas Schwab
  2 siblings, 1 reply; 15+ messages in thread
From: Joseph Myers @ 2021-08-12 16:19 UTC (permalink / raw)
  To: Patrick McGehearty; +Cc: gcc-patches, segher

On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:

> This patch resolves the failure of powerpc64 long double complex divide
> in native ibm long double format after the patch "Practical improvement
> to libgcc complex divide".

This description is not consistent with the patch.

__divkc3 should always be using IEEE binary128 format, not IBM long 
double.  If this code is being built for IBM long double, something is 
wrong somewhere else.

Using the DFmode values probably makes sense for IBM long double, but not 
for IEEE binary128.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:19 ` Joseph Myers
@ 2021-08-12 16:40   ` Patrick McGehearty
  2021-08-12 16:47     ` Joseph Myers
  0 siblings, 1 reply; 15+ messages in thread
From: Patrick McGehearty @ 2021-08-12 16:40 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, segher

The key code in _divkc3.c is:

#ifndef __LONG_DOUBLE_IEEE128__
#define RBIG   (__LIBGCC_DF_MAX__ / 2)
#define RMIN   (__LIBGCC_DF_MIN__)
#define RMIN2  (__LIBGCC_DF_EPSILON__)
#define RMINSCAL (1 / __LIBGCC_DF_EPSILON__)
#define RMAX2  (RBIG * RMIN2)
#else
#define RBIG   (__LIBGCC_TF_MAX__ / 2)
#define RMIN   (__LIBGCC_TF_MIN__)
#define RMIN2  (__LIBGCC_TF_EPSILON__)
#define RMINSCAL (1 / __LIBGCC_TF_EPSILON__)
#define RMAX2  (RBIG * RMIN2)
#endif

I added this code based on your comment of 4/20/2021:

---------
 > This file includes quad-float128.h, which does some remapping from TF to
 > KF depending on __LONG_DOUBLE_IEEE128__.
 >
 > I think you probably need to have a similar __LONG_DOUBLE_IEEE128__
 > conditional here.  If __LONG_DOUBLE_IEEE128__ is not defined, use
 > __LIBGCC_KF_* macros instead of __LIBGCC_TF_*; if 
__LONG_DOUBLE_IEEE128__
 > is defined, use __LIBGCC_TF_* as above.  (Unless the powerpc maintainers
 > say otherwise.)
---------
The KF version fails when in IBM128 mode while the DF version works
for that mode.

My understanding of ibm FP mode build procedure is minimal,
but it seems that the _divkc3.c routine is built for both IEEE128
and IBM128 modes.

- patrick



On 8/12/2021 11:19 AM, Joseph Myers wrote:
> On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:
>
>> This patch resolves the failure of powerpc64 long double complex divide
>> in native ibm long double format after the patch "Practical improvement
>> to libgcc complex divide".
> This description is not consistent with the patch.
>
> __divkc3 should always be using IEEE binary128 format, not IBM long
> double.  If this code is being built for IBM long double, something is
> wrong somewhere else.
>
> Using the DFmode values probably makes sense for IBM long double, but not
> for IEEE binary128.
>


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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:40   ` Patrick McGehearty
@ 2021-08-12 16:47     ` Joseph Myers
  2021-08-12 20:53       ` Segher Boessenkool
  2021-08-12 22:17       ` Patrick McGehearty
  0 siblings, 2 replies; 15+ messages in thread
From: Joseph Myers @ 2021-08-12 16:47 UTC (permalink / raw)
  To: Patrick McGehearty; +Cc: gcc-patches, segher

On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:

> > This file includes quad-float128.h, which does some remapping from TF to
> > KF depending on __LONG_DOUBLE_IEEE128__.
> >
> > I think you probably need to have a similar __LONG_DOUBLE_IEEE128__
> > conditional here.  If __LONG_DOUBLE_IEEE128__ is not defined, use
> > __LIBGCC_KF_* macros instead of __LIBGCC_TF_*; if __LONG_DOUBLE_IEEE128__
> > is defined, use __LIBGCC_TF_* as above.  (Unless the powerpc maintainers
> > say otherwise.)
> ---------
> The KF version fails when in IBM128 mode while the DF version works
> for that mode.

KFmode should always be IEEE binary128.  IFmode should always be IBM long 
double.  TFmode may be one or the other depending on command-line options.

"in IBM128 mode" should mean that the compiler defaults to long double 
being IBM long double and TFmode being IBM long double.  But in that mode, 
KFmode should still be IEEE binary128 and it should still be correct to 
use the KF constants in this file.

> My understanding of ibm FP mode build procedure is minimal,
> but it seems that the _divkc3.c routine is built for both IEEE128
> and IBM128 modes.

If built for IBM128 mode (i.e., compiler defaults to TFmode = IBM long 
double), it should still build a function __divkc3 which takes IEEE 
binary128 arguments and uses IEEE binary128 (KFmode) constants.

If you were changing the L_divtc3 case in libgcc2.c to use different 
constants in the case where TFmode is IBM long double, that would make 
sense to me.  It's changing an IEEE-only file for an IBM long double issue 
that doesn't make sense.  If this change causes some test using IBM long 
double to pass where it failed before, that indicates a build system 
problem somewhere.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:47     ` Joseph Myers
@ 2021-08-12 20:53       ` Segher Boessenkool
  2021-08-12 22:17       ` Patrick McGehearty
  1 sibling, 0 replies; 15+ messages in thread
From: Segher Boessenkool @ 2021-08-12 20:53 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Patrick McGehearty, gcc-patches

On Thu, Aug 12, 2021 at 04:47:42PM +0000, Joseph Myers wrote:
> On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:
> > My understanding of ibm FP mode build procedure is minimal,
> > but it seems that the _divkc3.c routine is built for both IEEE128
> > and IBM128 modes.
> 
> If built for IBM128 mode (i.e., compiler defaults to TFmode = IBM long 
> double), it should still build a function __divkc3 which takes IEEE 
> binary128 arguments and uses IEEE binary128 (KFmode) constants.

Apparently that is broken then :-(

> If you were changing the L_divtc3 case in libgcc2.c to use different 
> constants in the case where TFmode is IBM long double, that would make 
> sense to me.

This whole thing is there to *not* change generic code.


Segher

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:03 [PATCH v3] Fix for powerpc64 long double complex divide failure Patrick McGehearty
  2021-08-12 16:19 ` Joseph Myers
@ 2021-08-12 20:55 ` Segher Boessenkool
  2021-08-12 22:36 ` Andreas Schwab
  2 siblings, 0 replies; 15+ messages in thread
From: Segher Boessenkool @ 2021-08-12 20:55 UTC (permalink / raw)
  To: Patrick McGehearty; +Cc: gcc-patches

On Thu, Aug 12, 2021 at 04:03:13PM +0000, Patrick McGehearty wrote:
> This patch resolves the failure of powerpc64 long double complex divide
> in native ibm long double format after the patch "Practical improvement
> to libgcc complex divide".

[ etc. ]

Nothing in here says what has changed in v3.  Please do that in future
patches?


Segher

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:47     ` Joseph Myers
  2021-08-12 20:53       ` Segher Boessenkool
@ 2021-08-12 22:17       ` Patrick McGehearty
  2021-08-13 17:12         ` Joseph Myers
  1 sibling, 1 reply; 15+ messages in thread
From: Patrick McGehearty @ 2021-08-12 22:17 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, segher

I see I have more to learn about gcc's interactions with IEEE-128 format
vs IBM-128 format.

As we discovered here, using the IBM-128 version of LDBL_EPSILON will
not yield correct answers as currently coded.

If _divkc3.c is not intended to provide a version of complex divide
that handles IBM-128 format, then where should that option be handled?

Do I need add a special case for
#ifndef __LONG_DOUBLE_IEEE128__
in the complex divide code in libgcc/libgcc2.c?

And, for completeness, does gcc support LDBL for non-IEEE on
any platform besides IBM?

- patrick


On 8/12/2021 11:47 AM, Joseph Myers wrote:
> On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:
>
>>> This file includes quad-float128.h, which does some remapping from TF to
>>> KF depending on __LONG_DOUBLE_IEEE128__.
>>>
>>> I think you probably need to have a similar __LONG_DOUBLE_IEEE128__
>>> conditional here.  If __LONG_DOUBLE_IEEE128__ is not defined, use
>>> __LIBGCC_KF_* macros instead of __LIBGCC_TF_*; if __LONG_DOUBLE_IEEE128__
>>> is defined, use __LIBGCC_TF_* as above.  (Unless the powerpc maintainers
>>> say otherwise.)
>> ---------
>> The KF version fails when in IBM128 mode while the DF version works
>> for that mode.
> KFmode should always be IEEE binary128.  IFmode should always be IBM long
> double.  TFmode may be one or the other depending on command-line options.
>
> "in IBM128 mode" should mean that the compiler defaults to long double
> being IBM long double and TFmode being IBM long double.  But in that mode,
> KFmode should still be IEEE binary128 and it should still be correct to
> use the KF constants in this file.
>
>> My understanding of ibm FP mode build procedure is minimal,
>> but it seems that the _divkc3.c routine is built for both IEEE128
>> and IBM128 modes.
> If built for IBM128 mode (i.e., compiler defaults to TFmode = IBM long
> double), it should still build a function __divkc3 which takes IEEE
> binary128 arguments and uses IEEE binary128 (KFmode) constants.
>
> If you were changing the L_divtc3 case in libgcc2.c to use different
> constants in the case where TFmode is IBM long double, that would make
> sense to me.  It's changing an IEEE-only file for an IBM long double issue
> that doesn't make sense.  If this change causes some test using IBM long
> double to pass where it failed before, that indicates a build system
> problem somewhere.
>


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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 16:03 [PATCH v3] Fix for powerpc64 long double complex divide failure Patrick McGehearty
  2021-08-12 16:19 ` Joseph Myers
  2021-08-12 20:55 ` Segher Boessenkool
@ 2021-08-12 22:36 ` Andreas Schwab
  2021-08-13 17:22   ` Joseph Myers
  2 siblings, 1 reply; 15+ messages in thread
From: Andreas Schwab @ 2021-08-12 22:36 UTC (permalink / raw)
  To: Patrick McGehearty via Gcc-patches; +Cc: Patrick McGehearty, segher

On Aug 12 2021, Patrick McGehearty via Gcc-patches wrote:

> diff --git a/libgcc/config/rs6000/_divkc3.c b/libgcc/config/rs6000/_divkc3.c
> index a1d29d2..2b229c8 100644
> --- a/libgcc/config/rs6000/_divkc3.c
> +++ b/libgcc/config/rs6000/_divkc3.c
> @@ -38,10 +38,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  #endif
>  
>  #ifndef __LONG_DOUBLE_IEEE128__
> -#define RBIG   (__LIBGCC_KF_MAX__ / 2)
> -#define RMIN   (__LIBGCC_KF_MIN__)
> -#define RMIN2  (__LIBGCC_KF_EPSILON__)
> -#define RMINSCAL (1 / __LIBGCC_KF_EPSILON__)
> +#define RBIG   (__LIBGCC_DF_MAX__ / 2)
> +#define RMIN   (__LIBGCC_DF_MIN__)
> +#define RMIN2  (__LIBGCC_DF_EPSILON__)
> +#define RMINSCAL (1 / __LIBGCC_DF_EPSILON__)

How can it happen that __LONG_DOUBLE_IEEE128__ is not defined?  This
file is always compiled with -mfloat128 and this looks like dead code.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 22:17       ` Patrick McGehearty
@ 2021-08-13 17:12         ` Joseph Myers
  2021-08-26 15:51           ` Patrick McGehearty
  2021-08-26 17:04           ` Segher Boessenkool
  0 siblings, 2 replies; 15+ messages in thread
From: Joseph Myers @ 2021-08-13 17:12 UTC (permalink / raw)
  To: Patrick McGehearty; +Cc: gcc-patches, segher

On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:

> If _divkc3.c is not intended to provide a version of complex divide
> that handles IBM-128 format, then where should that option be handled?

That should be the function __divtc3.  (A single libgcc binary supports 
multiple long double formats, so libgcc function names referring to 
floating-point modes need to be understood as actually referring to a 
particular *format*, which may or may not correspond to the named *mode* 
depending on the compilation options used.  Thus, libgcc functions with 
"tf" or "tc" in their names, on configurations such as 
powerpc64le-linux-gnu that ever supported IBM long double, always refer to 
IBM long double.  It's up to the back end to ensure that, when building 
with TFmode = binary128, TCmode and KCmode division both get mapped to 
__divkc3 while ICmode division gets mapped to __divtc3; when building with 
TFmode = IBM long double, KCmode division should still be __divkc3, ICmode 
division should still be __divtc3, and TCmode division should be __divtc3 
in that case as well.)

> Do I need add a special case for
> #ifndef __LONG_DOUBLE_IEEE128__
> in the complex divide code in libgcc/libgcc2.c?

That macro is architecture-specific so shouldn't be tested there.  Doing 
things differently if __LIBGCC_TF_MANT_DIG__ == 106 would be reasonable 
(if it fixes the observed problem!), however; there are a few places in 
generic libgcc code that check for MANT_DIG == 106 to handle IBM long 
double differently.

> And, for completeness, does gcc support LDBL for non-IEEE on
> any platform besides IBM?

I believe TFmode is IEEE binary128 everywhere except for those powerpc 
configurations where it's IBM long double.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-12 22:36 ` Andreas Schwab
@ 2021-08-13 17:22   ` Joseph Myers
  2021-08-27  2:10     ` Michael Meissner
  0 siblings, 1 reply; 15+ messages in thread
From: Joseph Myers @ 2021-08-13 17:22 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Patrick McGehearty via Gcc-patches, segher

On Fri, 13 Aug 2021, Andreas Schwab wrote:

> On Aug 12 2021, Patrick McGehearty via Gcc-patches wrote:
> 
> > diff --git a/libgcc/config/rs6000/_divkc3.c b/libgcc/config/rs6000/_divkc3.c
> > index a1d29d2..2b229c8 100644
> > --- a/libgcc/config/rs6000/_divkc3.c
> > +++ b/libgcc/config/rs6000/_divkc3.c
> > @@ -38,10 +38,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >  #endif
> >  
> >  #ifndef __LONG_DOUBLE_IEEE128__
> > -#define RBIG   (__LIBGCC_KF_MAX__ / 2)
> > -#define RMIN   (__LIBGCC_KF_MIN__)
> > -#define RMIN2  (__LIBGCC_KF_EPSILON__)
> > -#define RMINSCAL (1 / __LIBGCC_KF_EPSILON__)
> > +#define RBIG   (__LIBGCC_DF_MAX__ / 2)
> > +#define RMIN   (__LIBGCC_DF_MIN__)
> > +#define RMIN2  (__LIBGCC_DF_EPSILON__)
> > +#define RMINSCAL (1 / __LIBGCC_DF_EPSILON__)
> 
> How can it happen that __LONG_DOUBLE_IEEE128__ is not defined?  This
> file is always compiled with -mfloat128 and this looks like dead code.

I think the answer there is that -mfloat128 serves to enable __float128, 
it doesn't change the long double ABI, which is what 
__LONG_DOUBLE_IEEE128__ refers to (that's what -mabi=ieeelongdouble does).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-13 17:12         ` Joseph Myers
@ 2021-08-26 15:51           ` Patrick McGehearty
  2021-08-26 16:34             ` Joseph Myers
  2021-08-26 16:57             ` Segher Boessenkool
  2021-08-26 17:04           ` Segher Boessenkool
  1 sibling, 2 replies; 15+ messages in thread
From: Patrick McGehearty @ 2021-08-26 15:51 UTC (permalink / raw)
  To: Joseph Myers; +Cc: gcc-patches, segher

Apologies in advance for the length of this reply.
Details seemed necessary to be sure we are on the same page
of understanding.

Aug 26, 2021 by Patrick McGehearty

The only IBM machines I have current access to are in the gcc fsf farm.

All studies were done on gcc135.fsffrance.org
Linux gcc135.osuosl.org 4.18.0-80.7.2.el7.ppc64le
/usr/bin/gcc version 4.8.5 20150623 (Red Hat 4.8.5-39) (GCC)
/usr/lib64/libc.so.6 linked to libc-2.17.so
yum list glibc shows the version 2.17-307.el7.1 while
version 2.17-324.el7_9 is available.

I'm concerned that the old environment on gcc135.fsffrance.org
is not showing me a typical build environment for IBM linux users
with more modern environments. I am also concerned that I may
not be setting my environment completely to use current
components.

- - - -

I built gcc from current upstream gcc srcs, replacing KF references
in libgcc/config/rs6000/_divkc3.c with DF references
and installed it in $HOME/usr.

For the following test purposes, I added the following to my local 
environment:
PATH=$HOME/usr/bin:$PATH
export PATH
LD_LIBRARY_PATH=$HOME/usr/lib64:$HOME/usr/lib
export LD_LIBRARY_PATH

The only complex divide routines in $HOME/usr/lib64/libgcc_s.so.1 are:
__divdc3, __divsc3, __divtc3

Compiling a simple c=a/b (for long double complex a,b,c)
yields a call to __divtc3
Compiling with -mabi=ieeelongdouble
yields a call to __divkc3

When I link the version with the call to __divkc3 into an executable,
the executable contains __divkc3. I just can't tell where the linker
is finding it.

- - - -
Using this environment, I tried building upstream gcc with __LIBGCC_KF_MAX__
and other #defines in  libgcc/config/rs6000/_divkc3.c

#ifndef __LONG_DOUBLE_IEEE128__
#define RBIG   (__LIBGCC_KF_MAX__ / 2)
#define RMIN   (__LIBGCC_KF_MIN__)
#define RMIN2  (__LIBGCC_KF_EPSILON__)
#define RMINSCAL (1 / __LIBGCC_KF_EPSILON__)
#define RMAX2  (RBIG * RMIN2)
#else

The KF case got errors of the sort:
‘__LIBGCC_KF_MAX__’ undeclared
‘__LIBGCC_KF_EPSILON__’ undeclared
‘__LIBGCC_KF_MIN__’ undeclared

These values were supposed to be created by gcc/c-family/c-cppbuiltin.c.
They depend on KF being part of
FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
{
   const char *name = GET_MODE_NAME (mode);
...
}

Apparently KF is not one of the mode names when building in this
environment. I've spent some time trying to understand where/how
the MODE_FLOAT class is constructed, but I have not been able to
pinpoint what's going wrong.

- - - -
When I replace the various _KF_ values with _DF_ values,
the compiler builds to completion.

In other testing I have confirmed that the rs6000/_divkc3.c code
does expect to be generating code for IEEE mode.

- - - - - - - - - - - - - - - - - - - - - - - -

When I compile/run the new test program cdivchkld.c with the old
compiler and -lm in the default IBM 128bit mode, it aborts as expected.
When I attempt to use the -mabi=ieeelongdouble -lm, the compiler
aborts with an internal error. Clearly ieeelongdouble is not
supported on this old compiler.

When I compile/run cdivchkld.c with my new compiler with
_DF_ values in _divkc3.c, it runs to completion without aborting.
That is why I believed rs6000/_divkc3.c affected the behavior
of IBM 128bit mode.

When I compile/link cdivchkld.c with -mabi=ieeelongdouble -lm
I get cdivchkld.c:(.text+0x3c4): undefined reference to `__fmaxieee128'
caused by the reference in the code to LDBL_MAX_EXP.

- - - - -
I suspect that part of the KF issue is that the general environment on
gcc135.fsffrance.org is relatively old. That is, it includes some
include files or libraries from before the KF/TF option for handling
IBM vs IEEE format was added to gcc. My use of $HOME/usr for PATH and
LD_LIBRARY_PATH may be inadequate for bypassing the older environment.
I don't have access to a current (el8 or later) IBM environment.

- - - - - - - - - - - - - - - - - - - - - - - -

With moderate study, I find I still don't really understand how the
make files are building both _divtc3.c and _divkc3.c as appropriate
for machines with/without HW support for IEEE128. It will be some time
before I could get enough ahead on my primary assignments to learn
what I need to know to gain that understanding.

I am concerned that in some IBM environments, the build process will
fall back to using the code in libgcc/libgcc2.c for IBM 128bit float
complex divide. In that case, the current 1/__LIBGCC_TF_EPSILON__
value will generate an infinity result which would be
suboptimal. Changing __LIBGCC_TF_EPSILON__ to __LIBGCC_DF_EPSILON__
for all platforms avoids the overflow without changing the final
answers. It only has the effect of doing some scaling without possible
overflow/underflow when it is not necessary.

I propose for my next patch I change libgcc/libgcc2.c to use
__LIBGCC_DF_EPSILON__ instead of __LIBGCC_TF_EPSILON__
in addition to the most recent changes in rs6000/_divkc3.c.
That set of changes will allow the upstream gcc to build on
the IBM build farm on fsf as well as allowing the new test results
to pass.

I'm not sure how else to approach this issue for a short term solution.


- Patrick McGehearty (patrick.mcgehearty@oracle.com)




On 8/13/2021 12:12 PM, Joseph Myers wrote:
> On Thu, 12 Aug 2021, Patrick McGehearty via Gcc-patches wrote:
>
>> If _divkc3.c is not intended to provide a version of complex divide
>> that handles IBM-128 format, then where should that option be handled?
> That should be the function __divtc3.  (A single libgcc binary supports
> multiple long double formats, so libgcc function names referring to
> floating-point modes need to be understood as actually referring to a
> particular *format*, which may or may not correspond to the named *mode*
> depending on the compilation options used.  Thus, libgcc functions with
> "tf" or "tc" in their names, on configurations such as
> powerpc64le-linux-gnu that ever supported IBM long double, always refer to
> IBM long double.  It's up to the back end to ensure that, when building
> with TFmode = binary128, TCmode and KCmode division both get mapped to
> __divkc3 while ICmode division gets mapped to __divtc3; when building with
> TFmode = IBM long double, KCmode division should still be __divkc3, ICmode
> division should still be __divtc3, and TCmode division should be __divtc3
> in that case as well.)
>
>> Do I need add a special case for
>> #ifndef __LONG_DOUBLE_IEEE128__
>> in the complex divide code in libgcc/libgcc2.c?
> That macro is architecture-specific so shouldn't be tested there.  Doing
> things differently if __LIBGCC_TF_MANT_DIG__ == 106 would be reasonable
> (if it fixes the observed problem!), however; there are a few places in
> generic libgcc code that check for MANT_DIG == 106 to handle IBM long
> double differently.
>
>> And, for completeness, does gcc support LDBL for non-IEEE on
>> any platform besides IBM?
> I believe TFmode is IEEE binary128 everywhere except for those powerpc
> configurations where it's IBM long double.
>


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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-26 15:51           ` Patrick McGehearty
@ 2021-08-26 16:34             ` Joseph Myers
  2021-08-26 16:57             ` Segher Boessenkool
  1 sibling, 0 replies; 15+ messages in thread
From: Joseph Myers @ 2021-08-26 16:34 UTC (permalink / raw)
  To: Patrick McGehearty; +Cc: gcc-patches, segher

On Thu, 26 Aug 2021, Patrick McGehearty via Gcc-patches wrote:

> The only complex divide routines in $HOME/usr/lib64/libgcc_s.so.1 are:
> __divdc3, __divsc3, __divtc3

Because no symbol versions are assigned to the KFmode symbols in the .ver 
files, so they are only exported from libgcc.a.

I think the exclusion of decimal FP arithmetic from shared libgcc is 
deliberate (both to faciliate using the libdfp version instead for some 
purposes, and maybe also to reduce libgcc_s library size and maybe TLS 
usage), but I don't know if the exclusion of KFmode arithmetic is also 
deliberate.

> When I link the version with the call to __divkc3 into an executable,
> the executable contains __divkc3. I just can't tell where the linker
> is finding it.

In libgcc.a, I expect.

> These values were supposed to be created by gcc/c-family/c-cppbuiltin.c.
> They depend on KF being part of
> FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
> {
>   const char *name = GET_MODE_NAME (mode);
> ...
> }
> 
> Apparently KF is not one of the mode names when building in this
> environment. I've spent some time trying to understand where/how
> the MODE_FLOAT class is constructed, but I have not been able to
> pinpoint what's going wrong.

So that's a key issue to resolve (but presumably this is working for some 
people building libgcc for powerpc64le).

> When I compile/link cdivchkld.c with -mabi=ieeelongdouble -lm
> I get cdivchkld.c:(.text+0x3c4): undefined reference to `__fmaxieee128'
> caused by the reference in the code to LDBL_MAX_EXP.

You need glibc 2.32 or later for __fmaxieee128 (and, generally, for IEEE 
long double support in glibc for powerpc64le).

> I am concerned that in some IBM environments, the build process will
> fall back to using the code in libgcc/libgcc2.c for IBM 128bit float
> complex divide. In that case, the current 1/__LIBGCC_TF_EPSILON__
> value will generate an infinity result which would be
> suboptimal. Changing __LIBGCC_TF_EPSILON__ to __LIBGCC_DF_EPSILON__
> for all platforms avoids the overflow without changing the final
> answers. It only has the effect of doing some scaling without possible
> overflow/underflow when it is not necessary.
> 
> I propose for my next patch I change libgcc/libgcc2.c to use
> __LIBGCC_DF_EPSILON__ instead of __LIBGCC_TF_EPSILON__

I think it's only appropriate to do that in the __LIBGCC_TF_MANT_DIG__ == 
106 case.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-26 15:51           ` Patrick McGehearty
  2021-08-26 16:34             ` Joseph Myers
@ 2021-08-26 16:57             ` Segher Boessenkool
  1 sibling, 0 replies; 15+ messages in thread
From: Segher Boessenkool @ 2021-08-26 16:57 UTC (permalink / raw)
  To: Patrick McGehearty; +Cc: Joseph Myers, gcc-patches

On Thu, Aug 26, 2021 at 10:51:51AM -0500, Patrick McGehearty wrote:
> I'm concerned that the old environment on gcc135.fsffrance.org
> is not showing me a typical build environment for IBM linux users
> with more modern environments.

It runs CentOS 7.  This is still supported.  It is getting long in the
tooth of course.

> I suspect that part of the KF issue is that the general environment on
> gcc135.fsffrance.org is relatively old. That is, it includes some
> include files or libraries from before the KF/TF option for handling
> IBM vs IEEE format was added to gcc. My use of $HOME/usr for PATH and
> LD_LIBRARY_PATH may be inadequate for bypassing the older environment.
> I don't have access to a current (el8 or later) IBM environment.

You need a newer glibc to get new features provided by that.  But,
everything should work with the older versions as well.


Segher

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-13 17:12         ` Joseph Myers
  2021-08-26 15:51           ` Patrick McGehearty
@ 2021-08-26 17:04           ` Segher Boessenkool
  1 sibling, 0 replies; 15+ messages in thread
From: Segher Boessenkool @ 2021-08-26 17:04 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Patrick McGehearty, gcc-patches

Hi!

On Fri, Aug 13, 2021 at 05:12:47PM +0000, Joseph Myers wrote:
> That should be the function __divtc3.  (A single libgcc binary supports 
> multiple long double formats, so libgcc function names referring to 
> floating-point modes need to be understood as actually referring to a 
> particular *format*, which may or may not correspond to the named *mode* 
> depending on the compilation options used.  Thus, libgcc functions with 
> "tf" or "tc" in their names, on configurations such as 
> powerpc64le-linux-gnu that ever supported IBM long double, always refer to 

That is historical as well.  Long ago the only 128-bit format was
double-double, and those mode names became part of the symbol names.
Changing this to __divic3 etc. would not really help.

Since the internal GCC symbol names are not really user-visible it does
not matter so much.


Segher

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

* Re: [PATCH v3] Fix for powerpc64 long double complex divide failure
  2021-08-13 17:22   ` Joseph Myers
@ 2021-08-27  2:10     ` Michael Meissner
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Meissner @ 2021-08-27  2:10 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Andreas Schwab, Patrick McGehearty via Gcc-patches, segher

On Fri, Aug 13, 2021 at 05:22:47PM +0000, Joseph Myers wrote:
> On Fri, 13 Aug 2021, Andreas Schwab wrote:
> 
> > On Aug 12 2021, Patrick McGehearty via Gcc-patches wrote:
> > How can it happen that __LONG_DOUBLE_IEEE128__ is not defined?  This
> > file is always compiled with -mfloat128 and this looks like dead code.
> 
> I think the answer there is that -mfloat128 serves to enable __float128, 
> it doesn't change the long double ABI, which is what 
> __LONG_DOUBLE_IEEE128__ refers to (that's what -mabi=ieeelongdouble does).

Yes this is correct.  The -mfloat128 enables the __float128 keyword.  It does
not change the default long double format.  Particularly before glibc 2.32 came
out we had some software packages that saw that we had had __float128, and
added functions to their library that took __float128 arguments.  Unfortunately
until the library had the support, it meant they would have references to
things not yet implemented.

We only enable -mfloat128 by default on 64-bit Linux systems.

The macro __LONG_DOUBLE_IEEE128__ is defined when long double uses the IEEE
128-bit format.

The macro __LONG_DOUBLE_IBM128__ is defined when long double uses the IBM
128-bit format.

The macro __LONG_DOUBLE_128__ is defined when one of the 128-bit long double
types (IBM or IEEE) is used for long double.

You can change the long double format with the -mabi=ieeelongdouble option or
by configuring the compiler with the --with-long-double-format=ieee
configuration option.

At present, only the C and C++ languages will work if you use the
-mabi=ieeelongdouble option and your GLIBC is at least 2.32.  This is because
the library support has been done to allow building both ibm128 and IEEE
128-bit functions in the library, and the compiler will use the appropriate
names based on the options.

Other languages such as fortran cannot be configured during compilation and you
must configure the compiler with the --with-long-double-format=ieee option.
This is because those libraries and compilers have not been modified to have
support for both 16-byte long double formats.  You will need to use at least
GLIBC 2.32.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

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

end of thread, other threads:[~2021-08-27  2:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 16:03 [PATCH v3] Fix for powerpc64 long double complex divide failure Patrick McGehearty
2021-08-12 16:19 ` Joseph Myers
2021-08-12 16:40   ` Patrick McGehearty
2021-08-12 16:47     ` Joseph Myers
2021-08-12 20:53       ` Segher Boessenkool
2021-08-12 22:17       ` Patrick McGehearty
2021-08-13 17:12         ` Joseph Myers
2021-08-26 15:51           ` Patrick McGehearty
2021-08-26 16:34             ` Joseph Myers
2021-08-26 16:57             ` Segher Boessenkool
2021-08-26 17:04           ` Segher Boessenkool
2021-08-12 20:55 ` Segher Boessenkool
2021-08-12 22:36 ` Andreas Schwab
2021-08-13 17:22   ` Joseph Myers
2021-08-27  2:10     ` Michael Meissner

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