[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_enc
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern. |
Date: |
Wed, 18 Sep 2019 12:08:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
Le 18/09/2019 à 11:59, Alex Bennée a écrit :
>
> Pierre Muller <address@hidden> writes:
>
>> Hi Thomas,
>>
>> I tried to use git format-patch -s below,
>> and change the commit message that appears below:
>>
>>
>> muller@gcc123:~/gnu/qemu/qemu$ git format-patch -s
>> a017dc6d43aaa4ffc7be40ae3adee4086be9cec2^
>> 0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
>> muller@gcc123:~/gnu/qemu/qemu$ cat
>> 0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
>
> It's best to send the patches directly (i.e. don't include them in a
> larger email). This is because when maintainers apply the email they end
> up with a bunch of additional stuff and the corrupting of subject line.
>
>> From a017dc6d43aaa4ffc7be40ae3adee4086be9cec2 Mon Sep 17 00:00:00 2001
>> From: Pierre Muller <address@hidden>
>> Date: Wed, 18 Sep 2019 08:04:19 +0000
>> Subject: [PATCH] Fix floatx80_invalid_encoding function for m68k cpu
>>
>> As m68k accepts different patterns for infinity,
>> and additional test for valid infinity must be added
>> for m68k cpu target.
>>
>> Signed-off-by: Pierre Muller <address@hidden>
>> ---
>> include/fpu/softfloat.h | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
>> index ecb8ba0114..dea24384e9 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 accepts different patterns 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
>> }
>
> As most of the test is the same we could rewrite this to:
>
> bool invalid = (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> #if defined (TARGET_M68K)
> invalid &= !floatx80_is_infinity(a)
> #endif
> return invalid;
>
> The compiler should be able to simplify the logic from there.
>
> Do we have any test cases that we could add to tests/tcg/m68k?
>
>>
>> #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL)
>
I think patch from Pierre doesn't treat all the problem and don't rely
on any specification.
I proposed a patch years ago that is closer to the hardware behaviour:
/*----------------------------------------------------------------------------
| 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.
*------------------------------------------------------------------------*/
return 0;
#else
return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
#endif
}
But in fact I think this kind of number should raise an exception.
I tried to do this in:
https://github.com/vivier/qemu-m68k/commit/5bd7742437a3c770373734add5ab452e8df4e621
but it needs more work and for the moment doesn't fix the problem Pierre is
trying to fix.
Thanks,
Laurent