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