Re: [Qemu-devel] [PATCH 08/12] ARM: Return correct result for single<->double conversion of NaN

[Nathan Froyd <froydnj@codesourcery.com>]

On Mon, Nov 29, 2010 at 07:25:18PM +0000, Peter Maydell wrote:
> On 29 November 2010 17:49, Nathan Froyd <froydnj@codesourcery.com> wrote:
> > On Tue, Nov 23, 2010 at 06:53:47PM +0000, Peter Maydell wrote:
> > As with other NaN-handling patches, I don't think the bit-twiddling here
> > is a good idea.  Having a float*_maybe_silence_nan function in softfloat
> > would be a better approach.
> 
> I guess this (like the other one you commented on) boils down to how
> you want to approach the boundary between qemu proper and the
> softfloat library. There are three approaches I can see:
> 
> (a) live with the softfloat API as it is, and add bit twiddling as
> necessary for particular target CPU special casing in the per-cpu
> functions (which is what I was doing here and with the 'is it a NaN?'
> function in the other patch)

Full disclosure: I did this sort of thing for PPC; see the DO_HANDLE_NAN
macro in op_helper.c.  I was young and thoughtless then and now I
am...well, older, anyway. :) I don't think it's the best approach: since
at least two CPUs now need NaN-silencing, we should provide something
generic.  And even if only one CPU requires it, I think it's better to
squirrel the logic away in fpu/.  float{32,64,80,128} should be opaque
things except for specialized cases.

(I can see an argument for CPU-confined bit twiddling to implement
things like float*_rsqrt_estimate or similar, where one function might
not work across CPU families due to precision requirements and so forth.
But we can cross that bridge when we come to it.)

> (b) add to and extend the softfloat API whenever you have some
> floating-point related thing it doesn't currently support (which I
> did with the "add conversions to int16_t" patch because it was
> a big chunk of bit twiddling, but which I felt was a bit invasive to
> do for this sort of minor tweak, especially since softfloat is a
> copy of a third-party library)

I think this is the best approach whenever possible.  I would not be too
worried about the third-party-ness of softfloat; it's extremely stable,
unlikely to change anytime in the near or far future, and we've already
extended in it non-trivial ways anyway.  (And would do so again if we
ever implemented, say, proper flush-to-zero denormal handling or IA64
register-precision floats--the former being more likely than the
latter. ;)

An example of where softfloat could be usefully extended and where we do
(a) sometimes is in production of CPU-default NaN values.  softfloat
knows all about this, yet there are #defines scattered about to provide
bit patterns because the softfloat API isn't extensive enough.

> (c) do something suboptimal where the softfloat API provides
> some-API-but-not-quite-the-ideal-API (which I'm not particularly
> keen on and is what I see the "is_nan() || is_signalling_nan()"
> approach as)

Yes, this is ugly.  Are you up for running:

  perl -p -i -e 's/float(\d+)_is_nan/float\1_is_quiet_nan/g' target-*/*.c

(and also carefully in fpu/*) or similar and moving the bit-twiddling
float_is_nan into fpu/?

-Nathan