| Message ID | 1294065273-30274-5-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On 3 January 2011 14:34, Aurelien Jarno <aurelien@aurel32.net> 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
On Mon, Jan 03, 2011 at 05:34:17PM +0000, Peter Maydell wrote: > On 3 January 2011 14:34, Aurelien Jarno <aurelien@aurel32.net> 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. I agree it should not make any change, but that it is conceptually wrong. I'll fix that in v2. > 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. > Probably to "aIsQuietNaN" to match the "aIsSignalingNaN" name. I'll send the v2 tomorrow.
diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h index 49e3cc2..4deb165 100644 --- a/fpu/softfloat-specialize.h +++ b/fpu/softfloat-specialize.h @@ -228,7 +228,7 @@ static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN, static float32 propagateFloat32NaN( float32 a, float32 b STATUS_PARAM) { flag aIsNaN, aIsSignalingNaN, bIsNaN, bIsSignalingNaN, aIsLargerSignificand; - bits32 av, bv, res; + bits32 av, bv; if ( STATUS(default_nan_mode) ) return float32_default_nan; @@ -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)) { @@ -258,12 +254,10 @@ static float32 propagateFloat32NaN( float32 a, float32 b STATUS_PARAM) if (pickNaN(aIsNaN, aIsSignalingNaN, bIsNaN, bIsSignalingNaN, aIsLargerSignificand)) { - res = bv; + return b; } else { - res = av; + return a; } - - return make_float32(res); } /*---------------------------------------------------------------------------- @@ -376,7 +370,7 @@ static float64 commonNaNToFloat64( commonNaNT a ) static float64 propagateFloat64NaN( float64 a, float64 b STATUS_PARAM) { flag aIsNaN, aIsSignalingNaN, bIsNaN, bIsSignalingNaN, aIsLargerSignificand; - bits64 av, bv, res; + bits64 av, bv; if ( STATUS(default_nan_mode) ) return float64_default_nan; @@ -385,15 +379,10 @@ static float64 propagateFloat64NaN( float64 a, float64 b STATUS_PARAM) aIsSignalingNaN = float64_is_signaling_nan( a ); bIsNaN = float64_is_quiet_nan( b ); bIsSignalingNaN = float64_is_signaling_nan( b ); + a = float64_maybe_silence_nan(a); + b = float64_maybe_silence_nan(b); av = float64_val(a); bv = float64_val(b); -#if SNAN_BIT_IS_ONE - av &= ~LIT64( 0x0008000000000000 ); - bv &= ~LIT64( 0x0008000000000000 ); -#else - av |= LIT64( 0x0008000000000000 ); - bv |= LIT64( 0x0008000000000000 ); -#endif if ( aIsSignalingNaN | bIsSignalingNaN ) float_raise( float_flag_invalid STATUS_VAR); if ((bits64)(av<<1) < (bits64)(bv<<1)) { @@ -406,12 +395,10 @@ static float64 propagateFloat64NaN( float64 a, float64 b STATUS_PARAM) if (pickNaN(aIsNaN, aIsSignalingNaN, bIsNaN, bIsSignalingNaN, aIsLargerSignificand)) { - res = bv; + return b; } else { - res = av; + return a; } - - return make_float64(res); } #ifdef FLOATX80 @@ -542,13 +529,9 @@ static floatx80 propagateFloatx80NaN( floatx80 a, floatx80 b STATUS_PARAM) aIsSignalingNaN = floatx80_is_signaling_nan( a ); bIsNaN = floatx80_is_quiet_nan( b ); bIsSignalingNaN = floatx80_is_signaling_nan( b ); -#if SNAN_BIT_IS_ONE - a.low &= ~LIT64( 0xC000000000000000 ); - b.low &= ~LIT64( 0xC000000000000000 ); -#else - a.low |= LIT64( 0xC000000000000000 ); - b.low |= LIT64( 0xC000000000000000 ); -#endif + a = floatx80_maybe_silence_nan(a); + a = floatx80_maybe_silence_nan(b); + if ( aIsSignalingNaN | bIsSignalingNaN ) float_raise( float_flag_invalid STATUS_VAR); if (a.low < b.low) { @@ -688,13 +671,9 @@ static float128 propagateFloat128NaN( float128 a, float128 b STATUS_PARAM) aIsSignalingNaN = float128_is_signaling_nan( a ); bIsNaN = float128_is_quiet_nan( b ); bIsSignalingNaN = float128_is_signaling_nan( b ); -#if SNAN_BIT_IS_ONE - a.high &= ~LIT64( 0x0000800000000000 ); - b.high &= ~LIT64( 0x0000800000000000 ); -#else - a.high |= LIT64( 0x0000800000000000 ); - b.high |= LIT64( 0x0000800000000000 ); -#endif + a = float128_maybe_silence_nan(a); + b = float128_maybe_silence_nan(b); + if ( aIsSignalingNaN | bIsSignalingNaN ) float_raise( float_flag_invalid STATUS_VAR); if (lt128(a.high<<1, a.low, b.high<<1, b.low)) {
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. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- fpu/softfloat-specialize.h | 55 +++++++++++++------------------------------ 1 files changed, 17 insertions(+), 38 deletions(-)