This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: mips SNaN/QNaN is swapped


Hi!

On Fri, 5 Apr 2013 23:55:37 +0100, "Maciej W. Rozycki" <macro@codesourcery.com> wrote:
> On Fri, 5 Apr 2013, Thomas Schwinge wrote:
> > > Index: gcc/config/fp-bit.c
> > > ===================================================================
> > > RCS file: /cvs/uberbaum/gcc/config/fp-bit.c,v
> > > retrieving revision 1.39
> > > diff -u -p -r1.39 fp-bit.c
> > > --- gcc/config/fp-bit.c 26 Jan 2003 10:06:57 -0000 1.39
> > > +++ gcc/config/fp-bit.c 1 Apr 2003 21:35:00 -0000
> > > @@ -210,7 +210,11 @@ pack_d ( fp_number_type *  src)
> > >        exp = EXPMAX;
> > >        if (src->class == CLASS_QNAN || 1)
> > >  	{
> > > +#ifdef QUIET_NAN_NEGATED
> > > +	  fraction |= QUIET_NAN - 1;
> > > +#else
> > >  	  fraction |= QUIET_NAN;
> > > +#endif

>  I think the intent of this code is to preserve a NaN's payload (it 
> certainly does for non-QUIET_NAN_NEGATED targets)

I agree.  For preserving the payload, both the unpack/pack code also has
to shift by NGARDS.

> Complementing the change above I think it will also make 
> sense to clear the qNaN bit when extracting a payload from fraction in 
> unpack_d as the class of a NaN being handled is stored separately.

I agree.

>  Also I find the "|| 1" clause in the condition immediately above the 
> pack_d piece concerned suspicious -- why is a qNaN returned for sNaN 
> input?  Likewise why are __thenan_sf, etc. encoded as sNaNs rather than 
> qNaNs?  Does anybody know?

I also stumbled over that, but for all these, I suppose the idea is that
when a sNaN is "arithmetically processed" (which includes datatype
conversion), an INVALID exception is to be raised (though, Â[fp-bit]
implements IEEE 754 format arithmetic, but does not provide a mechanism
[...] for generating or handling exceptionsÂ), and then converted into a
qNaN.

Also, I found that the bit to look at for distinguishing qNaN/sNaN is
defined wrongly for float.  Giving me some "interesting" test results...
;-)

Manual testing looks good.  Automated testing is still running; in case
nothing turns up, is this OK to check in?

libgcc/
	* fp-bit.c (unpack_d, pack_d): Properly preserve and restore a
	NaN's payload.
	* fp-bit.h [FLOAT] (QUIET_NAN): Correct value.

Index: libgcc/fp-bit.c
===================================================================
--- libgcc/fp-bit.c	(revision 402061)
+++ libgcc/fp-bit.c	(working copy)
@@ -214,11 +214,18 @@ pack_d (const fp_number_type *src)
   else if (isnan (src))
     {
       exp = EXPMAX;
+      /* Restore the NaN's payload.  */
+      fraction >>= NGARDS;
+      fraction &= QUIET_NAN - 1;
       if (src->class == CLASS_QNAN || 1)
 	{
 #ifdef QUIET_NAN_NEGATED
-	  fraction |= QUIET_NAN - 1;
+	  /* The quiet/signaling bit remains unset.  */
+	  /* Make sure the fraction has a non-zero value.  */
+	  if (fraction == 0)
+	    fraction |= QUIET_NAN - 1;
 #else
+	  /* Set the quiet/signaling bit.  */
 	  fraction |= QUIET_NAN;
 #endif
 	}
@@ -574,8 +581,10 @@ unpack_d (FLO_union_type * src, fp_number_type * d
 	    {
 	      dst->class = CLASS_SNAN;
 	    }
-	  /* Keep the fraction part as the nan number */
-	  dst->fraction.ll = fraction;
+	  /* Now that we know which kind of NaN we got, discard the
+	     quiet/signaling bit, but do preserve the NaN payload.  */
+	  fraction &= ~QUIET_NAN;
+	  dst->fraction.ll = fraction << NGARDS;
 	}
     }
   else
Index: libgcc/fp-bit.h
===================================================================
--- libgcc/fp-bit.h	(revision 402061)
+++ libgcc/fp-bit.h	(working copy)
@@ -190,7 +190,7 @@ typedef unsigned int UTItype __attribute__ ((mode
 #	define EXPBIAS 127
 #	define FRACBITS 23
 #	define EXPMAX (0xff)
-#	define QUIET_NAN 0x100000L
+#	define QUIET_NAN 0x400000L
 #	define FRAC_NBITS 32
 #	define FRACHIGH  0x80000000L
 #	define FRACHIGH2 0xc0000000L
@@ -298,7 +298,7 @@ typedef unsigned int UTItype __attribute__ ((mode
 /* numeric parameters */
 /* F_D_BITOFF is the number of bits offset between the MSB of the mantissa
    of a float and of a double. Assumes there are only two float types.
-   (double::FRAC_BITS+double::NGARDS-(float::FRAC_BITS-float::NGARDS))
+   (double::FRAC_BITS+double::NGARDS-(float::FRAC_BITS+float::NGARDS))
  */
 #define F_D_BITOFF (52+8-(23+7))
 


GrÃÃe,
 Thomas

Attachment: pgpVQDdgOWXgu.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]