Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding

[Laurent Vivier]

Le 15/06/2020 à 17:59, Fred Konrad a écrit :
> Missed this one sorry.
> 
> Le 6/12/20 à 10:31 AM, Laurent Vivier a écrit :
>> Le 28/04/2020 à 19:17, KONRAD Frederic a écrit :
>>> The MC68881 say about infinities (3.2.4):
>>>
>>> "*For the extended precision format, the most significant bit of the
>>> mantissa (the integer bit) is a don't care."
>>>
>>> https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf
>>>
>>> The m68k extended format is implemented with the floatx80 and
>>> floatx80_invalid_encoding currently treats 0x7fff00000000000000000000 as
>>> an invalid encoding.  This patch fixes floatx80_invalid_encoding so it
>>> accepts that the most significant bit of the mantissa can be 0.
>>>
>>> This bug can be revealed with the following code which pushes extended
>>> infinity on the stack as a double and then reloads it as a double.  It
>>> should normally be converted and read back as infinity and is currently
>>> read back as nan:
>>>
>>>          .global _start
>>>          .text
>>> _start:
>>>          lea val, %a0
>>>          lea fp, %fp
>>>          fmovex (%a0), %fp0
>>>          fmoved %fp0, %fp@(-8)
>>>          fmoved %fp@(-8), %fp0
>>> end:
>>>          bra end
>>>
>>> .align 0x4
>>> val:
>>>          .fill 1, 4, 0x7fff0000
>>>          .fill 1, 4, 0x00000000
>>>          .fill 1, 4, 0x00000000
>>> .align 0x4
>>>          .fill 0x100, 1, 0
>>> fp:
>>>
> 
> [...]
> 
>>
>> According to "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL" the explicit
>> integer bit is "Don't care" for signed infinite (a.high == 0x7FFF) (this
>> is the case this patch manages).
>>
>> But wit a zero exponent and a non zero mantissa, it's a denormal number,
>> and a signed zero has also a zero explicit integer bit but a zero
>> mantissa. (both cases are already managed in the existing code).
>>
>> with a non zero exponent less than the maximum value it's an unnormal
>> number.
>>
>> The denormal and unnormal numbers must be managed during the load
>> operation in the m68k TCG emulation to generate directly the FP_UNIMP
>> exception.
> 
> Is this already handled in the TCG code?

No, I have a skeleton with a workaround but if we enable the exception
the kernel crashes because the size of the frame saved in the stack by
fsave is not the one expected by the kernel (we save an IDLE frame and
not the UNIMP frame).

https://github.com/vivier/qemu-m68k/commit/c1297f61db283ccd592333f56907bd2961f1843c

I've also sent a patch similar to yours but disabling totally the
floatx80_invalid_encoding() check.

20200612140400.2130118-1-laurent@vivier.eu/">https://patchew.org/QEMU/20200612140400.2130118-1-laurent@vivier.eu/

Thanks,
Laurent