On Mon, Nov 29, 2010 at 07:25:18PM +0000, Peter Maydell wrote: > On 29 November 2010 17:49, Nathan Froyd <froy...@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