softfloat: fix floatx80_is_{quiet, signaling}_nan()

floatx80_is_{quiet,signaling}_nan() functions are incorrectly detecting
the type of NaN, depending on SNAN_BIT_IS_ONE, one of the two is
returning the correct value, and the other true for any kind of NaN.

This patch fixes that by applying the same kind of comparison as for
other float formats, but taking into account the explicit bit.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 fpu/softfloat-specialize.h |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

On 12 January 2011 13:59, Aurelien Jarno <aurelien@aurel32.net> wrote:

> @@ -494,7 +495,8 @@ int floatx80_is_quiet_nan( floatx80 a )
>  int floatx80_is_signaling_nan( floatx80 a )
>  {
>  #if SNAN_BIT_IS_ONE
> -    return ( ( a.high & 0x7FFF ) == 0x7FFF ) && (bits64) ( a.low<<1 );
> +    return ( ( a.high & 0x7FFF ) == 0x7FFF )
> +        && (LIT64( 0x8000000000000000 ) >= ((bits64) ( a.low<<1 )));
>  #else
>     bits64 aLow;

If a is {0x7ffff,0} (ie +inf) this will return true, which is wrong.
Do you want "<=" instead?

Actually, will
  return ((a.high & 0x7fff) == 0x7fff) && (a.low >= LIT64(0x4000000000000000));
do? Untested but I think it will do the right thing. I'm not sure
why this code has those bit64 casts, incidentally, since a.low is
already a uint64_t.

Also, maybe we should have a comment somewhere explaining
why this is different from the other NaN functions (ie that the
x80 format has an explicit bit and the others don't) ?

-- PMM

On Wed, Jan 12, 2011 at 03:05:10PM -0600, Peter Maydell wrote:
> On 12 January 2011 13:59, Aurelien Jarno <aurelien@aurel32.net> wrote:
> 
> > @@ -494,7 +495,8 @@ int floatx80_is_quiet_nan( floatx80 a )
> >  int floatx80_is_signaling_nan( floatx80 a )
> >  {
> >  #if SNAN_BIT_IS_ONE
> > -    return ( ( a.high & 0x7FFF ) == 0x7FFF ) && (bits64) ( a.low<<1 );
> > +    return ( ( a.high & 0x7FFF ) == 0x7FFF )
> > +        && (LIT64( 0x8000000000000000 ) >= ((bits64) ( a.low<<1 )));
> >  #else
> >     bits64 aLow;
> 
> If a is {0x7ffff,0} (ie +inf) this will return true, which is wrong.
> Do you want "<=" instead?

Correct, I swapped the operands at the last minute to match the other
functions, but without changing the sign.

> Actually, will
>   return ((a.high & 0x7fff) == 0x7fff) && (a.low >= LIT64(0x4000000000000000));
> do? Untested but I think it will do the right thing. I'm not sure

The explicit bit might be one for a NaN, so you should filter it first.

> why this code has those bit64 casts, incidentally, since a.low is
> already a uint64_t.

Don't know either, but as they were already there, I left them.

> Also, maybe we should have a comment somewhere explaining
> why this is different from the other NaN functions (ie that the
> x80 format has an explicit bit and the others don't) ?
> 

Good idea, will do.

On 13 January 2011 01:31, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Wed, Jan 12, 2011 at 03:05:10PM -0600, Peter Maydell wrote:
>> Actually, will
>>   return ((a.high & 0x7fff) == 0x7fff) && (a.low >= LIT64(0x4000000000000000));
>> do? Untested but I think it will do the right thing. I'm not sure
>
> The explicit bit might be one for a NaN, so you should filter it first.

Whoops, yes, explicit-bit-set but signalling-bit-clear would be
a false positive.

-- PMM