Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern.

[Thomas Huth]

On 17/09/2019 22.04, Pierre Muller wrote:
> 
> Hello,
> 
>   I submitted September 12. a bug report about wrong handling of
> infinity values for m68k emulator.
> 
> https://bugs.launchpad.net/qemu/+bug/1843651
> 
>   The analysis I made in the bug report is wrong, because
> I did not know that m68k FPU really uses 80-bit representations internally.
>   It thus seems now to me that the m68k specific floatx80_infinity_low
> corresponds to a different internal encoding of infinity on m68k floating
> point units, different from the one of the usual x87 floating point unit.
>   The main problem still remains that the function floatx80_invalid_encoding
> does not properly handle those m68k infinity patterns.
> 
>   The patch below seems to fix the reported problem on current git.
> 
> Pierre Muller
> Member of core development team of Free Pascal compiler
> 
> 
> From e053d3d07b1951faf0b4637ce1ec4194b8d952e5 Mon Sep 17 00:00:00 2001
> From: Pierre Muller <address@hidden>
> Date: Thu, 12 Sep 2019 08:10:48 +0000
> Subject: [PATCH]        * include/fpu/softfloat.h (floatx80_invalid_encoding):
>  Handle         m68k specific infinity pattern.

 Hi Pierre,

thanks a lot for the patch! But please note that the QEMU project has
some constraints for patches that have to be followed before a patch can
be applied.

Most important one: You need to provide a "Signed-off-by:" line in the
patch description to make sure that you agree with the Developer
Certificate Of Origin. See this URL for more details:

 https://wiki.qemu.org/Contribute/SubmitAPatch

Then it would be nice if you add some proper commit message to the patch
(something similar to the comment that you've added to the source code
would do the job, I guess).

And finally, please note that qemu-devel is a high traffic mailing list.
When sending patches, you best make sure to put some maintainers on CC:,
or your patch might get lost in the high traffic. You can either have a
look at the MAINTAINERS file in the main directory, or use the
scripts/get_maintainers.pl script on your patch to see who should be put
on CC:.

 Thanks,
  Thomas


> ---
>  include/fpu/softfloat.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ecb8ba0114..538c35767e 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -685,10 +685,17 @@ static inline int floatx80_is_any_nan(floatx80 a)
>  | 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.
> +| As m68k uses a different pattern for infinity, thus an additional test
> +| for valid infinity value must be added for m68k CPU.
>  
> *----------------------------------------------------------------------------*/
>  static inline bool floatx80_invalid_encoding(floatx80 a)
>  {
> +#if defined (TARGET_M68K)
> +    return ((a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0)
> +           && (! floatx80_is_infinity(a));
> +#else
>      return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> +#endif
>  }
> 
>  #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL)
> --
> 2.20.1
> 
>