Re: [Qemu-devel] [PATCH v3] Softfloat: Add support to softfloat to return floatxx_default_nan when, the corresponding target status flag is set.

[Aurelien Jarno]

Sorry to come back so long after, just found time to do some tests.

On Wed, Feb 09, 2011 at 06:56:50PM +0000, Peter Maydell wrote:
> On 9 February 2011 18:35, Aurelien Jarno <address@hidden> wrote:
> > On Tue, Feb 08, 2011 at 08:06:57PM +0000, Peter Maydell wrote:
> >> On 8 February 2011 18:53, Peter Maydell <address@hidden> wrote:
> >> Also, at the moment the commonNaNToFloatX(floatYToCommonNaN())
> >> idiom doesn't do anything to avoid signaling NaNs showing up in
> >> the output. On ARM this got fixed by having the helper.c wrappers
> >> call float*_maybe_silence_nan() but maybe we should do it
> >> in the generic softfloat code?
> >>
> >> ie instead of:
> >>
> >>     if ( mantissa )
> >>         return make_float32(
> >>             ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 ) );
> >>     else
> >>         return float32_default_nan;
> >>
> >> do:
> >>    float32 r = make_float32(
> >>             ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 ) );
> >
> > On an unrelated note, if we changes in this function, it might be a good
> > idea to use mantissa instead of a.high>>41. It's the same, but probably
> > easier to read.
> 
> Mmm, I noticed that (although I don't think I fixed it in my other
> patchset.)
> 
> >>    if (!mantissa) {
> >>       /* target specific, !SNAN_BIT_IS_ONE targets probably
> >>        * just set the QNAN bit (true for ARM, SPARC)
> >>        * others likely return the default NaN ?
> >>        */

For MIPS the current code is correct, it returns the default NaN in that
case. For at least PowerPC, SPARC and x86 setting the qNaN bit is the
thing to do. SH4 always returns the default NaN, so this code path is
never triggered.

> >>    } else {
> >>       return float32_maybe_silence_nan(r);

This part seems to work for at least PowerPC, SPARC and x86, but doesn't
match the MIPS implementation which doesn't silence NaNs during a
conversion (sNaN output for sNaN input). SH4 always returns the default
NaN, so this code path is never triggered.


> >>    }
> >>
> >> I'm pretty sure the existing code is wrong for SPARC, for instance,
> >> which is supposed to return a float32 qNaN with just the QNAN bit set
> >> if it is presented with a float64 signalling NaN all of whose non-zero
> >> mantissa bits are at the bottom end and don't make it into the float32.
> >> (ARM dodges a bullet here because as it happens our float32
> >> default_nan has only the QNAN bit set, but SPARC's has all the
> >> mantissa bits set.)
> >>
> >> Opinions?
> >>

The (!mantissa) change seems the way to got, OTOH it's probably better
to keep the (mantissa) case in target specific code.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net