Patchwork [01/20] softfloat: fix floatx80 handling of NaN

login
register
mail settings
Submitter Aurelien Jarno
Date April 18, 2011, 8:59 p.m.
Message ID <1303160412-8107-2-git-send-email-aurelien@aurel32.net>
Download mbox | patch
Permalink /patch/91848/
State New
Headers show

Comments

Aurelien Jarno - April 18, 2011, 8:59 p.m.
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 <aurelien@aurel32.net>
---
 fpu/softfloat-specialize.h |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)
Peter Maydell - April 19, 2011, 10:53 a.m.
On 18 April 2011 21:59, Aurelien Jarno <aurelien@aurel32.net> 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 <aurelien@aurel32.net>
> ---
>  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
Aurelien Jarno - April 20, 2011, 9:02 a.m.
On Tue, Apr 19, 2011 at 11:53:50AM +0100, Peter Maydell wrote:
> On 18 April 2011 21:59, Aurelien Jarno <aurelien@aurel32.net> 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 <aurelien@aurel32.net>
> > ---
> >  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?

I tested that on my hardware, on an Intel CPU, and it behaves like that.

> > @@ -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().
> 

Correct, good catch. Will fix that in v2.

Patch

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;
 }
 
@@ -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;
 }