Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[David Edelsohn]

libgcc
* config/rs6000/sfp-machine.h:
(_FP_DECL_EX): Declare _fpsr as a union of u64 and double.
(FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping
exceptions.
(FP_INIT_ROUNDMODE): Read the fpscr instead of writing
a mystery value.
(FP_ROUNDMODE): Update the usage of _fpscr.

Okay.

Thanks, David

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Paul E. Murphy]

Hi David,

Bill merged this into trunk last week.

Is it okay to backport this to GCC 5?

Thanks,
Paul

On 02/22/2016 05:51 PM, David Edelsohn wrote:
> libgcc
> * config/rs6000/sfp-machine.h:
> (_FP_DECL_EX): Declare _fpsr as a union of u64 and double.
> (FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping
> exceptions.
> (FP_INIT_ROUNDMODE): Read the fpscr instead of writing
> a mystery value.
> (FP_ROUNDMODE): Update the usage of _fpscr.
> 
> Okay.
> 
> Thanks, David
> 

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Bill Schmidt]

Hi David,

Please ignore this request.  I had asked Paul to do this, but was
confused that it relied on other patches that are not in GCC 5.  My
fault.

Thanks,
Bill

On Mon, 2016-02-29 at 11:33 -0600, Paul E. Murphy wrote:
> Hi David,
> 
> Bill merged this into trunk last week.
> 
> Is it okay to backport this to GCC 5?
> 
> Thanks,
> Paul
> 
> On 02/22/2016 05:51 PM, David Edelsohn wrote:
> > libgcc
> > * config/rs6000/sfp-machine.h:
> > (_FP_DECL_EX): Declare _fpsr as a union of u64 and double.
> > (FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping
> > exceptions.
> > (FP_INIT_ROUNDMODE): Read the fpscr instead of writing
> > a mystery value.
> > (FP_ROUNDMODE): Update the usage of _fpscr.
> > 
> > Okay.
> > 
> > Thanks, David
> > 

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Paul E. Murphy]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]



On 02/17/2016 08:37 PM, Alan Modra wrote:
>> +/* A set bit indicates an exception is trapping.  */
>> +# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
> 
> why then a shift here, since FP_EX_* are defined as the actual
> register bits?  Oh, I see.  FP_EX_* are the status bits, and you want
> the enable bits.  ie. bit 56 rather than bit 34, bit 57 rather than
> bit 35 and so on (bits numbered from 0 as msb).  A comment to that
> effect might reduce head scratching.

Thanks for taking a look Alan, and Joseph.  I've expanded the comment
to clarify the otherwise mysterious shift with the updated patch.

libgcc
	* config/rs6000/sfp-machine.h:
	(_FP_DECL_EX): Declare _fpsr as a union of u64 and double.
	(FP_TRAPPING_EXCEPTIONS): Return a bitmask of trapping
	exceptions.
	(FP_INIT_ROUNDMODE): Read the fpscr instead of writing
	a mystery value.
	(FP_ROUNDMODE): Update the usage of _fpscr.


[-- Attachment #2: ppc_ieee128_sfp_fix_rev2.patch --]
[-- Type: text/x-patch, Size: 1681 bytes --]

diff --git a/libgcc/config/rs6000/sfp-machine.h b/libgcc/config/rs6000/sfp-machine.h
index 75d5e1a..8dd8ea3 100644
--- a/libgcc/config/rs6000/sfp-machine.h
+++ b/libgcc/config/rs6000/sfp-machine.h
@@ -130,9 +130,13 @@ void __sfp_handle_exceptions (int);
     if (__builtin_expect (_fex, 0))		\
       __sfp_handle_exceptions (_fex);		\
   } while (0);
-/* A set bit indicates an exception is masked and a clear bit indicates it is
-   trapping.  */
-# define FP_TRAPPING_EXCEPTIONS (~_fpscr & (FP_EX_ALL >> 22))
+
+/* The FP_EX_* bits track whether the exception has occurred.  This macro
+   must set the FP_EX_* bits of those exceptions which are configured to
+   trap.  The FPSCR bit which indicates this is 22 ISA bits above the
+   respective FP_EX_* bit.  Note, the ISA labels bits from msb to lsb,
+   so 22 ISA bits above is 22 bits below when counted from the lsb.  */
+# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
 
 # define FP_RND_NEAREST	0x0
 # define FP_RND_ZERO	0x1
@@ -141,16 +145,16 @@ void __sfp_handle_exceptions (int);
 # define FP_RND_MASK	0x3
 
 # define _FP_DECL_EX \
-  unsigned long long _fpscr __attribute__ ((unused)) = FP_RND_NEAREST
+  union { unsigned long long i; double d; } _fpscr __attribute__ ((unused)) = \
+	 { .i = FP_RND_NEAREST }
 
 #define FP_INIT_ROUNDMODE			\
   do {						\
-    __asm__ __volatile__ ("mtfsf 255, %0"	\
-			  :			\
-			  : "f" (_fpscr));	\
+    __asm__ __volatile__ ("mffs %0"		\
+			  : "=f" (_fpscr.d));	\
   } while (0)
 
-# define FP_ROUNDMODE	(_fpscr & FP_RND_MASK)
+# define FP_ROUNDMODE	(_fpscr.i & FP_RND_MASK)
 #endif	/* !__FLOAT128__ */
 
 /* Define ALIASNAME as a strong alias for NAME.  */

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Alan Modra]

On Wed, Feb 17, 2016 at 05:40:01PM -0600, Paul E. Murphy wrote:
> - FP_INIT_ROUNDMODE writes junk to the fpscr. I assume this should be
>   reading the fpscr and initializing the local rounding mode variable
>   declared via _FP_DECL_EX.

Yeah, looks that way.

> - FP_TRAPPING_EXCEPTIONS evaluates to zero where used. It seems like it
>   should return a bit field of FP_EX_* bits indicating which trap is
>   enabled. Likewise, when these bits are set in the fpscr, the trap is
>   enabled.

Yes, but

> +/* A set bit indicates an exception is trapping.  */
> +# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)

why then a shift here, since FP_EX_* are defined as the actual
register bits?  Oh, I see.  FP_EX_* are the status bits, and you want
the enable bits.  ie. bit 56 rather than bit 34, bit 57 rather than
bit 35 and so on (bits numbered from 0 as msb).  A comment to that
effect might reduce head scratching.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Joseph Myers]

I have no comments on this patch beyond again suggesting that it would be 
desirable for some future float128 GCC patch to enable as many as possible 
of the x86 float128 tests (that is, tests of float128 anywhere in the GCC 
testsuite that currently are restricted to x86, x86_64 and ia64 targets or 
some subset thereof) also for powerpc (with appropriate dg- directives to 
allow target-specific compilation options to be added, say dg-add-options 
float128).

While some of those tests are inapplicable to powerpc (e.g. 
gcc.dg/torture/float128-extendxf-underflow.c because it uses XFmode, and 
float128-mul-underflow.c, float128-truncdf-underflow.c, 
float128-truncsf-underflow.c because they test for after-rounding tininess 
detection, which is correct for x86 / ia64 but not for powerpc), others 
should apply to powerpc just as to x86 / ia64, and the tests do test 
various aspects of exceptions / rounding modes handling (and other issues, 
e.g. conversions in fp-int-convert-float128*), and it would not surprise 
me if they would have shown up some of the issues fixed by this patch.

-- 
Joseph S. Myers
joseph@codesourcery.com

PPC libgcc IEEE128 soft-fp exception/rounding fixes

[Paul E. Murphy]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

Hi all,

I am fairly new to IBM and recently appointed maintainer of libdfp,
and work on glibc on ppc. This is my first foray into libgcc.

libdfp implements many common transcendental functions, overrides
type conversions between decimal float and other GCC types with more
optimized variants for dpd encoding.

While investigating some rounding issues with conversions, I tried out
the recent IEEE128 soft-fp support for PPC using a supporting compiler,
while building some of the soft-fp parts locally.

I ran into the following issues which I have attempted to correct with
the attached patch for rs6000/sfp-machine.h:

- FP_INIT_ROUNDMODE writes junk to the fpscr. I assume this should be
  reading the fpscr and initializing the local rounding mode variable
  declared via _FP_DECL_EX.

- FP_TRAPPING_EXCEPTIONS evaluates to zero where used. It seems like it
  should return a bit field of FP_EX_* bits indicating which trap is
  enabled. Likewise, when these bits are set in the fpscr, the trap is
  enabled.


libgcc
	* config/rs6000/sfp-machine.h:
	(_FP_DECL_EX): Declare _fpsr as union of u64 and double.
	(FP_TRAPPING_EXCEPTIONS): Remove this, FP_HANDLE_EXCEPTIONS
	will do them implicitly later on.
	(FP_INIT_ROUNDMODE): Read the fpscr instead of writing
	mystery value.
	(FP_ROUNDMODE): Update type of _fpscr.

[-- Attachment #2: ppc_ieee128_sfp_fix.patch --]
[-- Type: text/x-patch, Size: 1369 bytes --]

diff --git a/libgcc/config/rs6000/sfp-machine.h b/libgcc/config/rs6000/sfp-machine.h
index 75d5e1a..4bc0040 100644
--- a/libgcc/config/rs6000/sfp-machine.h
+++ b/libgcc/config/rs6000/sfp-machine.h
@@ -130,9 +130,9 @@ void __sfp_handle_exceptions (int);
     if (__builtin_expect (_fex, 0))		\
       __sfp_handle_exceptions (_fex);		\
   } while (0);
-/* A set bit indicates an exception is masked and a clear bit indicates it is
-   trapping.  */
-# define FP_TRAPPING_EXCEPTIONS (~_fpscr & (FP_EX_ALL >> 22))
+
+/* A set bit indicates an exception is trapping.  */
+# define FP_TRAPPING_EXCEPTIONS ((_fpscr.i << 22) & FP_EX_ALL)
 
 # define FP_RND_NEAREST	0x0
 # define FP_RND_ZERO	0x1
@@ -141,16 +141,16 @@ void __sfp_handle_exceptions (int);
 # define FP_RND_MASK	0x3
 
 # define _FP_DECL_EX \
-  unsigned long long _fpscr __attribute__ ((unused)) = FP_RND_NEAREST
+  union { unsigned long long i; double d; } _fpscr __attribute__ ((unused)) = \
+	 { .i = FP_RND_NEAREST }
 
 #define FP_INIT_ROUNDMODE			\
   do {						\
-    __asm__ __volatile__ ("mtfsf 255, %0"	\
-			  :			\
-			  : "f" (_fpscr));	\
+    __asm__ __volatile__ ("mffs %0"		\
+			  : "=f" (_fpscr.d));	\
   } while (0)
 
-# define FP_ROUNDMODE	(_fpscr & FP_RND_MASK)
+# define FP_ROUNDMODE	(_fpscr.i & FP_RND_MASK)
 #endif	/* !__FLOAT128__ */
 
 /* Define ALIASNAME as a strong alias for NAME.  */