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 Iain!

Joseph pointed out that I didn't include you, the libgcc and fp-bit
maintainer, in my mails' recipient lists.  I'm waiting for approval for
the following changes:

On Mon, 22 Apr 2013 11:52:23 +0200, I wrote:
> 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: pgpROzuxwKOYH.pgp
Description: PGP signature


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