Re: [Qemu-devel] [PATCH 1/7] softfloat: Fix sNaN handling in FP conversion operations

[Maciej W. Rozycki]

On Fri, 6 Feb 2015, Maciej W. Rozycki wrote:

> > >> I think this means that:
> > >>  (1) we want to put handling of silencing the signaling NaNs
> > >>  into the NaN conversion functions themselves (since it's
> > >>  too late to do it correctly once the commonNaNtoFloat16
> > >>  function has thrown away data)
> > >>  (2) that handling needs to be target specific, as most details
> > >>  of quiet vs signaling NaNs are
> > >>
> > >> I note in passing that the float*ToCommonNaN and commonNaNToFloat*
> > >> functions are used only in the back-to-back call sequence of
> > >>    return commonNaNToFloatX(floatYToCommonNaN(...))
> > >> for handling the NaN inputs on FP-to-FP conversion.
> > >
> > >  I believe my proposal addresses your concerns in a burden-free way for
> > > target maintainers who look after processors that implement the IEEE 754
> > > standard as it stands.
> > 
> > I don't, which is why I made the comment above. If you're going to
> > fix up NaN handling in the float-to-float conversion routines we
> > should do it in a way that lets us handle the behaviour of
> > target CPUs we care about.
> 
>  No argument about maintaining correct emulation where it already is such, 
> sure.  Please note that `float64_maybe_silence_nan' and friends are 
> already target-specific, which should generally let us deal with things, 
> and I am sort of surprised ARM sets certain rules for sNaN silencing for 
> conversions and different ones for other arithmetic operations.  Or is it 
> really that an input sNaN's sign bit is propagated in the single weird 
> corner case only?

 OK, I see where the issue is -- this is specifically about the case where 
dropping trailing significand bits of an sNaN in the process of format 
conversion would result in an infinity being produced.  This is handled by 
`commonNaNToFloat*' functions, but not in a way the ARM architecture 
requires.  As a preexisting problem this has nothing to do with my 
proposal and neither my proposal makes things worse, though I indeed 
missed the opportunity to correct it.

 I've thought of two general solutions:

1. Given that `*_default_nan' are now functions make them accept the sign 
   bit as input and propagate it to output if required by a given 
   implementation.  Conversions would pass the input sign bit while
   arithmetic operations would hardcode it to 0.

2. Quieten an sNaN being converted in the wider format operated on.  For a 
   widening conversion it would be the output format just as in the 
   original patch proposed.  For a narrowing conversion it would be the 
   input format.

I'm leaning towards the second option as being more elegant and robust.

 So to give a specific example, `float32_to_float64' would have:

        if (aSig) {
            float64 nan;
            nan = commonNaNToFloat64(float32ToCommonNaN(a, status), status);
            return float64_maybe_silence_nan(nan);
        }

but `float64_to_float32' would have:

        if (aSig) {
            float64 nan;
            nan = float64_maybe_silence_nan(a);
            return commonNaNToFloat32(float64ToCommonNaN(nan, status), status);
        }

instead.  This does the right thing as far as NaN quietening is concerned 
and avoids the `!mantissa' case triggering in `commonNaNToFloat32'.

 Based on the lone contents of fpu/softfloat-specialize.h this solution 
looks correct to me, in particular for the SNAN_BIT_IS_ONE case -- are you 
aware of any processor architectures for which this approach would not 
produce the result required?

 I've had a look at target-arm/helper.c and target-arm/helper-a64.c and 
AFAICT the hacks they do for NaN quietening do not follow the architecture 
specification in narrowing conversions (they merely reproduce what I 
proposed with the original patch and therefore do not preserve the sign 
bit in the case concerned), so the approach I outlined above will actually 
progress these targets as well, and these hacks can then be removed.  
This solution also meets the IEEE 754-2008 requirement for the way NaN 
quietening is done where the preferred NaN encoding is implemented.

  Maciej