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]

On Mon, Feb 21, 2011 at 05:44:49PM +0100, Aurelien Jarno wrote:
> 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.
> 

Actually that part seems to depends on the implementations. I have seen
3 cases:
- sNaN are not silenced
- sNaN are silenced
- default NaN is returned

The MIPS manual doesn't specify this case precisely, however it seems to 
rule out the first case, which seems to be a bug of the silicon:

  "An SNaN is never produced as a result value."

So I am fine if we go with float32_maybe_silence_nan()

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