Re: [Qemu-devel] [PATCH 4/6] softfloat: use float{32, 64, x80, 128}_maybe_silence_nan()

[Peter Maydell]

On 3 January 2011 14:34, Aurelien Jarno <address@hidden> wrote:
> Use float{32,64,x80,128}_maybe_silence_nan() instead of toggling the
> sNaN bit manually. This allow per target implementation of sNaN to qNaN
> conversion.

> @@ -237,15 +237,11 @@ static float32 propagateFloat32NaN( float32 a, float32 
> b STATUS_PARAM)
>     aIsSignalingNaN = float32_is_signaling_nan( a );
>     bIsNaN = float32_is_quiet_nan( b );
>     bIsSignalingNaN = float32_is_signaling_nan( b );
> +    a = float32_maybe_silence_nan(a);
> +    b = float32_maybe_silence_nan(b);
>     av = float32_val(a);
>     bv = float32_val(b);
> -#if SNAN_BIT_IS_ONE
> -    av &= ~0x00400000;
> -    bv &= ~0x00400000;
> -#else
> -    av |= 0x00400000;
> -    bv |= 0x00400000;
> -#endif
> +
>     if ( aIsSignalingNaN | bIsSignalingNaN ) float_raise( float_flag_invalid 
> STATUS_VAR);
>
>     if ((bits32)(av<<1) < (bits32)(bv<<1)) {

The place you've put the maybe_silence_nan() calls means that
the calculation of aIsLargerSignificand is done based on the silenced
values rather than on the input values, which I think is wrong (it
is inconsistent with the *isSignalingNaN and *isNaN flags).

It doesn't make any difference in practice, because the only thing
that uses it is the x87 rules, and they only look at aIsLargerSignificand
if both a and b are the same type of NaN, in which case by x87
silencing rules the comparison gives the same result whether it's
before or after silencing. But I think conceptually it would be better
to call float*_maybe_silence_nan() after the calculation of
aIsLargerSignificand.

Unrelated trivial cleanup which I've just noticed and which you might
want to throw into v2 of this patchset:
>     bIsNaN = float32_is_quiet_nan( b );

...these variables would be better named as aIsQNan, bIsQNaN in
these propagate* functions.

-- PMM