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
