mips SNaN/QNaN is swapped

Maciej W. Rozycki macro@codesourcery.com
Sat Apr 6 21:40:00 GMT 2013


On Fri, 5 Apr 2013, Thomas Schwinge wrote:

> As I understand it (and I may add that this is the first time ever I'm
> looking at soft-fp internals), that appears to be a bug in soft-fp, in
> this very code added ten years ago ;-), which is invoked by means of
> _df_to_tf.o:__extenddftf2 for this conversion operation.  (Forgive my
> ignorance of MIPS ISA floating-point details, and not looking it up
> myself at this late time of day -- why use soft-fp for that; isn't there
> anything in hardware available to do it?)

 There's no direct MIPS hardware support for any FP data type wider than 
double.

> > 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
> >  	}
> >      }
> >    else if (isinf (src))
> > @@ -521,7 +525,11 @@ unpack_d (FLO_union_type * src, fp_numbe
> >        else
> >  	{
> >  	  /* Nonzero fraction, means nan */
> > +#ifdef QUIET_NAN_NEGATED
> > +	  if ((fraction & QUIET_NAN) == 0)
> > +#else
> >  	  if (fraction & QUIET_NAN)
> > +#endif
> >  	    {
> >  	      dst->class = CLASS_QNAN;
> >  	    }
> 
> With the fix applied, we get the expected result:
> 
>     7fbfffff
>     7ff7ffff ffffffff
>     7ff7ffff e0000000
>     7ff7ffff ffffffff
>     7fff7fff ffffffff ffffffff ffffffff
>     7fff7fff ffffffff ffffffff ffffffff
>     7fff7fff ffffffff ffffffff ffffffff
>     7fff7fff ffffffff ffffffff ffffffff
>     7fff7fff ffffffff ffffffff ffffffff
> 
> Automated testing is still running; in case nothing turns up, does this
> look OK to check in?
> 
> Index: libgcc/fp-bit.c
> ===================================================================
> --- libgcc/fp-bit.c	(revision 402061)
> +++ libgcc/fp-bit.c	(working copy)
> @@ -217,6 +217,9 @@ pack_d (const fp_number_type *src)
>        if (src->class == CLASS_QNAN || 1)
>  	{
>  #ifdef QUIET_NAN_NEGATED
> +	  /* Mask out the quiet/signaling bit.  */
> +	  fraction &= ~QUIET_NAN;
> +	  /* Set the remainder of the fraction to a non-zero value.  */
>  	  fraction |= QUIET_NAN - 1;
>  #else
>  	  fraction |= QUIET_NAN;

 I think the intent of this code is to preserve a NaN's payload (it 
certainly does for non-QUIET_NAN_NEGATED targets), so corrected code 
should IMHO look like:

#ifdef QUIET_NAN_NEGATED
	  fraction &= QUIET_NAN - 1;
	  if (fraction == 0)
	    fraction |= QUIET_NAN - 1;
#else
	  fraction |= QUIET_NAN;
#endif

or suchalike -- making sure a NaN is not accidentally converted to 
infinity where qNaNs are denoted by a zero bit (in which case the 
canonical qNaN is returned instead -- the code above is correct for MIPS 
legacy targets where the canonical qNaN has an all-ones payload; can't 
speak of HP-PA).  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.

 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?

  Maciej



More information about the Gcc-patches mailing list