diff mbox series

Fix for powerpc64 long double complex divide failure

Message ID 1625779471-11996-1-git-send-email-patrick.mcgehearty@oracle.com
State New
Headers show
Series Fix for powerpc64 long double complex divide failure | expand

Commit Message

Patrick McGehearty July 8, 2021, 9:24 p.m. UTC 
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".
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101104

The new code uses the following macros which are intended to be mapped
to appropriate values according to the underlying hardware representation.

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 exponent
field for IBM 128-bit floating point values is the same size as
the exponent field for IBM 64-bit floating point values. However
the EPSILON field is considerably different. Due to how 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.

In addition, the gcc support for the KF fields (IBM native long double
format) does not exist on older gcc compilers such as the default
compilers on the gcc compiler farm. That adds build complexity
for users who's environment is only a few years out of date.
Instead of just replacing the use of KF_EPSILON with DF_ESPILON,
we replace all uses of KF_* with DF_*.

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

libgcc/
	* config/rs6000/_divkc3.c (RBIG, RMIN, RMIN2, RMINSCAL, RMAX2):
	Fix long double complex divide for native IBM 128-bit
---
 libgcc/config/rs6000/_divkc3.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Patrick McGehearty July 20, 2021, 4:25 p.m. UTC | #1 
Ping...

The fix is minimal (four lines changed).
I recognize that those familiar with IBM 128-bit floating
point precision is a select set of people.
On the plus side, tests fail without the patch and pass with the patch.

- patrick


On 7/8/2021 4:24 PM, 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".
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101104
>
> The new code uses the following macros which are intended to be mapped
> to appropriate values according to the underlying hardware representation.
>
> 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 exponent
> field for IBM 128-bit floating point values is the same size as
> the exponent field for IBM 64-bit floating point values. However
> the EPSILON field is considerably different. Due to how 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.
>
> In addition, the gcc support for the KF fields (IBM native long double
> format) does not exist on older gcc compilers such as the default
> compilers on the gcc compiler farm. That adds build complexity
> for users who's environment is only a few years out of date.
> Instead of just replacing the use of KF_EPSILON with DF_ESPILON,
> we replace all uses of KF_* with DF_*.
>
> The change has been tested on gcc135.fsffrance.org and gains the
> expected improvements in accuracy for long double complex divide.
>
> libgcc/
> 	* config/rs6000/_divkc3.c (RBIG, RMIN, RMIN2, RMINSCAL, RMAX2):
> 	Fix long double complex divide 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)
Segher Boessenkool July 20, 2021, 9:34 p.m. UTC | #2 
On Tue, Jul 20, 2021 at 11:25:29AM -0500, Patrick McGehearty via Gcc-patches wrote:
> Ping...
> 
> The fix is minimal (four lines changed).
> I recognize that those familiar with IBM 128-bit floating
> point precision is a select set of people.
> On the plus side, tests fail without the patch and pass with the patch.

Hi!

In the future, please Cc: the relevant maintainers.  Your patch will be
seen much quicker and much more reliably if you do.  I'll handle it now.


Segher
Segher Boessenkool July 20, 2021, 10:53 p.m. UTC | #3 
Hi!

On Thu, Jul 08, 2021 at 09:24:31PM +0000, 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".
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101104

> The MAX and MIN values have only modest changes since the exponent
> field for IBM 128-bit floating point values is the same size as
> the exponent field for IBM 64-bit floating point values.

Finite double-double values have two exponent fields, one for each DP
half that together make up the number.  You are referring to the first
one here.

> However
> the EPSILON field is considerably different. Due to how 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.

The representable values of double-double and those of IEEE QP float are
not a subset of each other in either direction.  Since a number in
double-double is essentially the sum of two DP float numbers, two such
sums numbers can be very close together, compared to the magnitude of
them (the epsilon is equal to the minimum non-zero value).

> In addition, the gcc support for the KF fields (IBM native long double
> format)

KFmode is IEEE QP float, always.  IFmode is IBM extended double, always.
TFmode can be either.

> does not exist on older gcc compilers such as the default
> compilers on the gcc compiler farm. That adds build complexity
> for users who's environment is only a few years out of date.

CentOS 7 is from 2014.  It's about the oldest we support at all.

> libgcc/

You should say
	PR target/101104
> 	* config/rs6000/_divkc3.c (RBIG, RMIN, RMIN2, RMINSCAL, RMAX2):
> 	Fix long double complex divide for native IBM 128-bit

End sentences (like lines in a changelog) with a full stop, too.

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

What *is* your long double?  It should always be IEEE QP float in this
file!  And it is for me.  So what did you do differently?


Segher
diff mbox series

Patch

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)