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

[Aurelien Jarno]

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.

> 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.

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.

That said, it is possible to check for a NaN before and after the call
to the FP conversion routine, but that will slow down the emulation.


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