Re: [Qemu-devel] [PATCH v3 02/16] softloat: disable floatx80_invalid_encoding() for m68k

[Peter Maydell]

On 7 February 2017 at 00:59, Laurent Vivier <address@hidden> wrote:
> According to the comment, this definition of invalid encoding is given
> by intel developer's manual, and doesn't work with the behavior
> of 680x0 FPU.
>
> CC: Andreas Schwab <address@hidden>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
>  fpu/softfloat.c         | 31 +++++++++++++++++++++++++++++++
>  include/fpu/softfloat.h | 15 ---------------
>  2 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index c295f31..f95b19f 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -4799,6 +4799,37 @@ int float64_unordered_quiet(float64 a, float64 b, 
> float_status *status)
>  }
>
>  
> /*----------------------------------------------------------------------------
> +| Return whether the given value is an invalid floatx80 encoding.
> +| Invalid floatx80 encodings arise when the integer bit is not set, but
> +| the exponent is not zero. The only times the integer bit is permitted to
> +| be zero is in subnormal numbers and the value zero.
> +| This includes what the Intel software developer's manual calls pseudo-NaNs,
> +| pseudo-infinities and un-normal numbers. It does not include
> +| pseudo-denormals, which must still be correctly handled as inputs even
> +| if they are never generated as outputs.
> +*----------------------------------------------------------------------------*/
> +static inline bool floatx80_invalid_encoding(floatx80 a)
> +{
> +#if defined(TARGET_M68K)
> +    
> /*-------------------------------------------------------------------------
> +    |  M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL
> +    |  1.6.2 Denormalized Numbers
> +    |  Since the extended-precision data format has an explicit integer bit,
> +    |  a number can be formatted with a nonzero exponent, less than the 
> maximum
> +    |  value, and a zero integer bit.  The IEEE 754 standard does not define 
> a
> +    |  zero integer bit. Such a number is an unnormalized number. Hardware 
> does
> +    |  not directly support denormalized and unnormalized numbers, but
> +    |  implicitly supports them by trapping them as unimplemented data types,
> +    |  allowing efficient conversion in software.
> +    
> *------------------------------------------------------------------------*/

This comment says that numbers of the form that this function
identifies (zero integer bit and nonzero exponent) are
supposed to trap on m68k. That suggests that the target/m68k code
is going to need to catch this case before it calls softfloat
code. So (a) we don't care what the softfloat code does
with these representations because they'll never get to it
and (b) the m68k code could probably use floatx80_invalid_encoding()
as a function to identify where it needs to trap, which it
can't do if it's moved out of softfloat.h.

Either way this change doesn't seem right given what the
comment describes...

thanks
-- PMM