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:
>> + /* ARM requires that S<->D conversion of any kind of NaN generates
>> + * a quiet NaN by forcing the most significant frac bit to 1.
>> + */
>> + if (float64_is_signaling_nan(r)) {
>> + return make_float64(float64_val(r) | (1LL << 51));
>> + }
> 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)
(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)
(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)
My original patchset tends to (a) except where (b) is clearly
more sensible; if people would prefer (b) all the time I'm happy
to do things that way; (c) doesn't seem very attractive to me
and I would rather do (b) in those situations.
-- PMM
