| Message ID | 1303160412-8107-2-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
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
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.
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; }
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(-)