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

[Peter Maydell]

On 6 February 2015 at 14:37, Maciej W. Rozycki <address@hidden> wrote:
> On Thu, 5 Feb 2015, Peter Maydell wrote:
>> Have you looked at the other architectures that use these
>> functions to convert float values to see what their NaN
>> handling semantics are? I agree about what the spec says,
>> but we may well have targets which are not fully IEEE
>> compliant and rely on the current semantics (x86 springs to
>> mind).
>
>  I think the IEEE 754 standard-compliant behaviour shouldn't be left to
> target helpers, it would bound to cause discrepancies and chaos.  I
> wouldn't expect every target maintainer to be fluent in IEEE 754 arcana,
> or even have access to the standard's normative document.
>
>  What I think would make sense here is instead of say `float32_to_float64'
> making a call to `float64_maybe_silence_nan' directly, we'd have a static
> inline function or a macro called say `float64_convert_silence_nan'
> invoked where the former is in my patch proposed.  That function in turn
> would call the former function by default and which could be overridden by
> something else for individual targets that require it.

This still doesn't work for ARM, because you can't do the
silencing after conversion. You have to have a way for
targets to put in target-specific behaviour directly in the
64-to-32 conversion process, before data is lost in the narrowing.

>  And I think we shouldn't be making our decisions based on x86 despite its
> ubiquity -- it is, especially as the x87 implementation is concerned, the
> earliest attempt to implement IEEE 754 with all the oddities resulting.
> Or more specifically not even that but more of an FP implementation that
> (as of the 8087 FPU) Intel wanted to become the basis of the upcoming IEEE
> 754 standard, and mostly succeeded in principle, but not in some details,
> some of which were later corrected with the 80287 and 80387
> implementations, and some of which had to stay, for compatibility reasons.

My point is that you mustn't regress other targets. So either you
need to retain the same behaviour for targets which don't specifically
add new code/flags/whatever, or you need to go through and look up
all the other architecture specifications to check that you haven't
broken them (ie that any changes you make are actually improving their
accuracy). This patch isn't doing the first of those, so you need
to do the second...

>  Besides are you sure?

I was asking if you'd checked all the targets whose behaviour you're
changing, not making a definite "you have broken x86" statement.
x86 is just the most likely because as you say it is the weirdest
and least compliant.

>> Also in this area, ARM has a problem where we don't give the
>> correct answers for fp-to-fp conversions from a higher precision
>> to a lower precision where the input is a signaling NaN whose
>> non-zero mantissa bits are all at the low end such that the
>> truncation of the mantissa into the lower precision format
>> would drop them all. The result is the wrong value of quiet NaN
>> (we return the default NaN, which has the sign bit clear, but the
>> FPConvertNaN pseudocode specifies that we should effectively get
>> the default NaN but with the same sign bit as the input SNaN).
>>
>> 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.

thanks
-- PMM