Re: [PATCH v1] softfloat: Silence signaling NaN when converting to/from float128

[David Hildenbrand <david@redhat.com>]

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

-- 
Thanks,

David / dhildenb