diff mbox series

Fix PR bootstrap/100327 (_divkf3.c) on PowerPC

Message ID 20210429161342.GA4837@ibm-toto.the-meissners.org
State New
Headers show
Series Fix PR bootstrap/100327 (_divkf3.c) on PowerPC | expand

Commit Message

Michael Meissner April 29, 2021, 4:13 p.m. UTC
Fix PR bootstrap/100327 (_divkf3.c) on PowerPC.

This patch fixes the PowerPC _divkf3.c module to use the appropriate
FLT128 constants if long double is not IEEE 128-bit.

I have tested this patch by doing a bootstrap on a little endian power9
system running Linux.  Can I check this into the trunk?

gcc/
2021-04-29  Michael Meissner  <meissner@linux.ibm.com>

	PR bootstrap/100327
	* config/rs6000/_divkf3.c (RBIG, RMIN, RMIN2, RMINSCAL): Use the
	appropriate FLT128 constant if long double is not IEEE 128-bit.

Comments

Joseph Myers April 29, 2021, 4:31 p.m. UTC | #1
On Thu, 29 Apr 2021, Michael Meissner via Gcc-patches wrote:

> Fix PR bootstrap/100327 (_divkf3.c) on PowerPC.
> 
> This patch fixes the PowerPC _divkf3.c module to use the appropriate
> FLT128 constants if long double is not IEEE 128-bit.
> 
> I have tested this patch by doing a bootstrap on a little endian power9
> system running Linux.  Can I check this into the trunk?

Why aren't the __LIBGCC_KF_* macros defined?  If there's a bug in the 
logic to define macros for all floating-point modes supported by libgcc, 
it should be fixed there.  If there's some reason that can't work, the 
commit message needs to explain in more detail.
Michael Meissner April 29, 2021, 4:37 p.m. UTC | #2
On Thu, Apr 29, 2021 at 04:31:50PM +0000, Joseph Myers wrote:
> On Thu, 29 Apr 2021, Michael Meissner via Gcc-patches wrote:
> 
> > Fix PR bootstrap/100327 (_divkf3.c) on PowerPC.
> > 
> > This patch fixes the PowerPC _divkf3.c module to use the appropriate
> > FLT128 constants if long double is not IEEE 128-bit.
> > 
> > I have tested this patch by doing a bootstrap on a little endian power9
> > system running Linux.  Can I check this into the trunk?
> 
> Why aren't the __LIBGCC_KF_* macros defined?  If there's a bug in the 
> logic to define macros for all floating-point modes supported by libgcc, 
> it should be fixed there.  If there's some reason that can't work, the 
> commit message needs to explain in more detail.

As Richard points out, the default libgcc_floating_mode_supported_p returns
false for these types.  If you add a target hook to return true for these
types, the c-cppbuiltin.c then aborts because there is no suffix for these
types.  There is no target hook to define strings to add new suffixes (or
prefixes).
Joseph Myers April 29, 2021, 4:48 p.m. UTC | #3
On Thu, 29 Apr 2021, Michael Meissner via Gcc-patches wrote:

> On Thu, Apr 29, 2021 at 04:31:50PM +0000, Joseph Myers wrote:
> > On Thu, 29 Apr 2021, Michael Meissner via Gcc-patches wrote:
> > 
> > > Fix PR bootstrap/100327 (_divkf3.c) on PowerPC.
> > > 
> > > This patch fixes the PowerPC _divkf3.c module to use the appropriate
> > > FLT128 constants if long double is not IEEE 128-bit.
> > > 
> > > I have tested this patch by doing a bootstrap on a little endian power9
> > > system running Linux.  Can I check this into the trunk?
> > 
> > Why aren't the __LIBGCC_KF_* macros defined?  If there's a bug in the 
> > logic to define macros for all floating-point modes supported by libgcc, 
> > it should be fixed there.  If there's some reason that can't work, the 
> > commit message needs to explain in more detail.
> 
> As Richard points out, the default libgcc_floating_mode_supported_p returns
> false for these types.  If you add a target hook to return true for these
> types, the c-cppbuiltin.c then aborts because there is no suffix for these
> types.  There is no target hook to define strings to add new suffixes (or
> prefixes).

Presumably the code works with the __FLT128_* macros as used in your 
patch.  Those should expand to constants using the f128 suffix.  So there 
*is* an available suffix here (f128).  Why doesn't the code there, which 
checks all the available _FloatN and _FloatNx types for any with the right 
machine mode, find that suffix?

(The target hook indeed can't return true for modes that don't have any 
corresponding suffix, e.g. IFmode.  The modes for which it returns true 
may need to depend on the options passed to the compiler.)
Michael Meissner April 29, 2021, 9:42 p.m. UTC | #4
On Thu, Apr 29, 2021 at 04:48:07PM +0000, Joseph Myers wrote:
> On Thu, 29 Apr 2021, Michael Meissner via Gcc-patches wrote:
> 
> > On Thu, Apr 29, 2021 at 04:31:50PM +0000, Joseph Myers wrote:
> > > On Thu, 29 Apr 2021, Michael Meissner via Gcc-patches wrote:
> > > 
> > > > Fix PR bootstrap/100327 (_divkf3.c) on PowerPC.
> > > > 
> > > > This patch fixes the PowerPC _divkf3.c module to use the appropriate
> > > > FLT128 constants if long double is not IEEE 128-bit.
> > > > 
> > > > I have tested this patch by doing a bootstrap on a little endian power9
> > > > system running Linux.  Can I check this into the trunk?
> > > 
> > > Why aren't the __LIBGCC_KF_* macros defined?  If there's a bug in the 
> > > logic to define macros for all floating-point modes supported by libgcc, 
> > > it should be fixed there.  If there's some reason that can't work, the 
> > > commit message needs to explain in more detail.
> > 
> > As Richard points out, the default libgcc_floating_mode_supported_p returns
> > false for these types.  If you add a target hook to return true for these
> > types, the c-cppbuiltin.c then aborts because there is no suffix for these
> > types.  There is no target hook to define strings to add new suffixes (or
> > prefixes).
> 
> Presumably the code works with the __FLT128_* macros as used in your 
> patch.  Those should expand to constants using the f128 suffix.  So there 
> *is* an available suffix here (f128).  Why doesn't the code there, which 
> checks all the available _FloatN and _FloatNx types for any with the right 
> machine mode, find that suffix?

Good point.  I will submit a new patch after this message.

> (The target hook indeed can't return true for modes that don't have any 
> corresponding suffix, e.g. IFmode.  The modes for which it returns true 
> may need to depend on the options passed to the compiler.)

Yes that is the tricky part.  Due to decisions made earlier, if long double
uses the IEEE 128-bit format, the _Float128/__float128 support uses TFmode and
not KFmode.  But I was able to build bootstrap compiler with normal IBM
extended long double, and a non-bootstrap compiler with IEEE 128-bit.  I will
build the bootstrap IEEE 128-bit compiler shortly.
diff mbox series

Patch

--- /home/meissner/tmp/gcc-tmp/dDj7PV__divkc3.c	2021-04-29 10:32:37.669452599 -0500
+++ libgcc/config/rs6000/_divkc3.c	2021-04-29 10:27:10.388470401 -0500
@@ -38,10 +38,10 @@  see the files COPYING3 and COPYING.RUNTI
 #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   (__FLT128_MAX__ / 2)
+#define RMIN   (__FLT128_MIN__)
+#define RMIN2  (__FLT128_EPSILON__)
+#define RMINSCAL (1 / __FLT128_EPSILON__)
 #define RMAX2  (RBIG * RMIN2)
 #else
 #define RBIG   (__LIBGCC_TF_MAX__ / 2)