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