Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

J. Mayer a écrit :
> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> The current softfloat implementation changes qNaN into sNaN when 
>>>>>> converting between formats, for no reason. The attached patch fixes
>>>>>> that. It also fixes an off-by-one in the extended double precision
>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
>>>>>> long.
>>>>>>
>>>>>> With this patch applied all the glibc 2.7 floating point tests
>>>>>> are successfull on MIPS and MIPSEL.
>>>>> FYI, I posted a similar patch and haven't had time to get back to it.
>>>>> Andreas reminded me that we need to make sure at least one mantissa
>>>>> bit is set.  If we're confident that the common NaN format will
>>>>> already have some bit other than the qnan/snan bit set, this is fine;
>>>>> otherwise, we might want to forcibly set some other mantissa bit.
>>>>>
>>>> Please find an updated patch below. I have tried to match real x86, MIPS,
>>>> HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.
>>> It's a good idea to fix NaN problems here but in my opinion, it's a bad
>>> idea to have target dependant code here. This code should implement IEEE
>>> behavior. Target specific behavior / deviations from the norm has to be
>> Has Thiemo already said, there is no IEEE behavior. If you look at the
>> IEEE 754 document you will see that it has requirements on what should
>> be supported by an IEEE compliant FPU, but has very few requirements on
>> the implementation.
>>
> 
> OK.
> 
>>> implemented in target specific code. As targets have to check the
>>> presence of a NaN to update the FP flags, it seems that uglyifying this
>>> code with target specific hacks is pointless. If the target code do not
>>> check the presence of a NaN, that means that it does not implement
>>> precise FPU emulation, then there's no need to have specific code to
>>> return a precise value (I mean target dependant) from the generic code,
>>> imho.
>> I actually know very few CPU that check for NaN in general. They check
>> for sNaN as required by IEEE 754, but rarely for qNaN as their purpose
>> is exactly to be propagated through all FPU operations as a normal FP 
>> number would be.
> 
> CPU do check QNaNs because most of them update a specific flag that can
> be checked to know there was a NaN seen during FPU operations. I don't
> know for all FPU, but I can see that the PowerPC gives me 4 bits that
> give the class of the last FPU result and I guess you have those kind of
> flags in most implementations.

This is not the case of MIPS for example.

>> Anyway there is no way to do that in the target specific code *after 
>> the conversion*, as the detection of a mantissa being nul when 
>> converting from double to single precision can only be done when both
>> values are still known. In other words when the value is not fixed 
>> during the conversion, the value 0x7f800000 can either be infinity or a
>> conversion of NaN from double to single precision, and thus is it not
>> possible to fix the value afterwards in the target specific code.
> 
> I don't say you have to return an infinity when the argument is a qNaN.
> I just say you have to return a qNaN in a generic way.  Just return sign
> | 0x7f800000 | mantissa, which is the more generic form and seems to me
> to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

> PowerPC) that specify that the result have to be equal to the operand
> (in the single precision format, of course) in such a case. This is
> simpler, it ensures that any target could then detect the presence of a
> NaN, know which one, and can then adjust the value according to its
> specification if needed.
> I then still can'tl see any reason of having target specific code in
> that area.

Ok, let's give an example then. On MIPS let's say you want to convert
0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
the right become 0, so you have to generate a new value. As you
proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
routines. This value has to be converted to 0x7fbfffff in the MIPS
target code.

However, that doesn't work because 0x7ff8000000000000 (sNaN) will also
be converted to 0x7fc00000 in the softfloat routines. The MIPS target
code will convert that into 0x7bffffff, which is wrong because it is a
qNaN, and because the mantissa has been changed while it should have not.

In short this is not possible to fix a value while it is already
converted to single precision, because all the possible 2^32 values have
a signification.

-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net