Re: [Qemu-devel] [PATCH 01/20] softfloat: fix floatx80 handling of NaN

[Peter Maydell]

On 18 April 2011 21:59, Aurelien Jarno <address@hidden> wrote:
> The floatx80 format uses an explicit bit that should be taken into account
> when converting to and from commonNaN format.
>
> When converting to commonNaN, the explicit bit should be removed if it is
> a 1, and a default NaN should be used if it is 0.
>
> When converting from commonNan, the explicit bit should be added.
>
> Signed-off-by: Aurelien Jarno <address@hidden>
> ---
>  fpu/softfloat-specialize.h |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
> index b110187..fb2b5b4 100644
> --- a/fpu/softfloat-specialize.h
> +++ b/fpu/softfloat-specialize.h
> @@ -603,9 +603,15 @@ static commonNaNT floatx80ToCommonNaN( floatx80 a 
> STATUS_PARAM)
>     commonNaNT z;
>
>     if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid 
> STATUS_VAR);
> -    z.sign = a.high>>15;
> -    z.low = 0;
> -    z.high = a.low;
> +    if ( a.low >> 63 ) {
> +        z.sign = a.high >> 15;
> +        z.low = 0;
> +        z.high = a.low << 1;
> +    } else {
> +        z.sign = floatx80_default_nan_high >> 15;
> +        z.low = 0;
> +        z.high = floatx80_default_nan_low << 1;
> +    }
>     return z;
>  }

The intel manuals don't seem to define what a number with non-zero exponent
field but explicit bit clear actually means. Presumably this (generate a
default NaN) is what the hardware does if you try to convert such a thing
to float64?

> @@ -624,10 +630,11 @@ static floatx80 commonNaNToFloatx80( commonNaNT a 
> STATUS_PARAM)
>         return z;
>     }
>
> -    if (a.high)
> -        z.low = a.high;
> -    else
> +    if (a.high) {
> +        z.low = LIT64( 0x8000000000000000 ) | a.high >> 1;
> +    } else {
>         z.low = floatx80_default_nan_low;
> +    }
>     z.high = ( ( (uint16_t) a.sign )<<15 ) | 0x7FFF;
>     return z;
>  }

I think the condition here should be "if (a.high >> 1)" -- otherwise we
might construct an infinity instead (explicit bit 1 but all fraction bits 0).
Also we are keeping the sign of the input even if we return the default
NaN. It might be better to start with
 uint64_t mantissa = a.high >> 1;
and then roll the 'mantissa == 0' check into the default_nan_mode if().

-- PMM