| Message ID | 20210505104952.5632-1-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] softfloat: Silence signaling NaN when converting to/from float128 | expand |
On 05.05.21 12:49, David Hildenbrand wrote: > We forgot to silence the NaN, just as we already do for the other > conversions. > > Found by comparing the result of running randomly generated FP instructions > under s390x/tcg and comparing against the result on real HW. > > Unfortunately, test cases like f32_to_f128 cannot be unlocked yet as > some expected values (with NaN) are wrongly calculated. > > Cc: Aurelien Jarno <aurelien@aurel32.net> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: "Alex Bennée" <alex.bennee@linaro.org> > Cc: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > fpu/softfloat.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index 67cfa0fd82..e9f2117a6d 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -4924,7 +4924,9 @@ float128 float32_to_float128(float32 a, float_status *status) > aSign = extractFloat32Sign( a ); > if ( aExp == 0xFF ) { > if (aSig) { > - return commonNaNToFloat128(float32ToCommonNaN(a, status), status); > + float128 res = commonNaNToFloat128(float32ToCommonNaN(a, status), > + status); > + return float128_silence_nan(res, status); > } > return packFloat128( aSign, 0x7FFF, 0, 0 ); > } > @@ -5229,7 +5231,9 @@ float128 float64_to_float128(float64 a, float_status *status) > aSign = extractFloat64Sign( a ); > if ( aExp == 0x7FF ) { > if (aSig) { > - return commonNaNToFloat128(float64ToCommonNaN(a, status), status); > + float128 res = commonNaNToFloat128(float64ToCommonNaN(a, status), > + status); > + return float128_silence_nan(res, status); > } > return packFloat128( aSign, 0x7FFF, 0, 0 ); > } > @@ -6665,7 +6669,9 @@ float32 float128_to_float32(float128 a, float_status *status) > aSign = extractFloat128Sign( a ); > if ( aExp == 0x7FFF ) { > if ( aSig0 | aSig1 ) { > - return commonNaNToFloat32(float128ToCommonNaN(a, status), status); > + float32 res = commonNaNToFloat32(float128ToCommonNaN(a, status), > + status); > + return float32_silence_nan(res, status); > } > return packFloat32( aSign, 0xFF, 0 ); > } > @@ -6699,7 +6705,9 @@ float64 float128_to_float64(float128 a, float_status *status) > aSign = extractFloat128Sign( a ); > if ( aExp == 0x7FFF ) { > if ( aSig0 | aSig1 ) { > - return commonNaNToFloat64(float128ToCommonNaN(a, status), status); > + float64 res = commonNaNToFloat64(float128ToCommonNaN(a, status), > + status); > + return float64_silence_nan(res, status); > } > return packFloat64( aSign, 0x7FF, 0 ); > } > This problem is also fixed after Richard's rework: https://lkml.kernel.org/r/20210508014802.892561-1-richard.henderson@linaro.org
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 67cfa0fd82..e9f2117a6d 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -4924,7 +4924,9 @@ float128 float32_to_float128(float32 a, float_status *status) aSign = extractFloat32Sign( a ); if ( aExp == 0xFF ) { if (aSig) { - return commonNaNToFloat128(float32ToCommonNaN(a, status), status); + float128 res = commonNaNToFloat128(float32ToCommonNaN(a, status), + status); + return float128_silence_nan(res, status); } return packFloat128( aSign, 0x7FFF, 0, 0 ); } @@ -5229,7 +5231,9 @@ float128 float64_to_float128(float64 a, float_status *status) aSign = extractFloat64Sign( a ); if ( aExp == 0x7FF ) { if (aSig) { - return commonNaNToFloat128(float64ToCommonNaN(a, status), status); + float128 res = commonNaNToFloat128(float64ToCommonNaN(a, status), + status); + return float128_silence_nan(res, status); } return packFloat128( aSign, 0x7FFF, 0, 0 ); } @@ -6665,7 +6669,9 @@ float32 float128_to_float32(float128 a, float_status *status) aSign = extractFloat128Sign( a ); if ( aExp == 0x7FFF ) { if ( aSig0 | aSig1 ) { - return commonNaNToFloat32(float128ToCommonNaN(a, status), status); + float32 res = commonNaNToFloat32(float128ToCommonNaN(a, status), + status); + return float32_silence_nan(res, status); } return packFloat32( aSign, 0xFF, 0 ); } @@ -6699,7 +6705,9 @@ float64 float128_to_float64(float128 a, float_status *status) aSign = extractFloat128Sign( a ); if ( aExp == 0x7FFF ) { if ( aSig0 | aSig1 ) { - return commonNaNToFloat64(float128ToCommonNaN(a, status), status); + float64 res = commonNaNToFloat64(float128ToCommonNaN(a, status), + status); + return float64_silence_nan(res, status); } return packFloat64( aSign, 0x7FF, 0 ); }
We forgot to silence the NaN, just as we already do for the other conversions. Found by comparing the result of running randomly generated FP instructions under s390x/tcg and comparing against the result on real HW. Unfortunately, test cases like f32_to_f128 cannot be unlocked yet as some expected values (with NaN) are wrongly calculated. Cc: Aurelien Jarno <aurelien@aurel32.net> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: "Alex Bennée" <alex.bennee@linaro.org> Cc: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: David Hildenbrand <david@redhat.com> --- fpu/softfloat.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)