diff mbox

[1/7] softfloat: Fix sNaN handling in FP conversion operations

Message ID alpine.DEB.1.10.1412082125030.19155@tp.orcam.me.uk
State New
Headers show

Commit Message

Maciej W. Rozycki Dec. 9, 2014, 1:54 a.m. UTC
Fix sNaN handling in floating-point format conversion operations, that 
are classified by the IEEE 754-2008 standard as general-computational 
operations [1]:

"5.4 formatOf general-computational operations

"5.4.2 Conversion operations for floating-point formats and decimal 
character sequences

"Implementations shall provide the following formatOf conversion 
operations from all supported floating-point formats to all supported 
floating-point formats, as well as conversions to and from decimal 
character sequences.  These operations shall not propagate non-canonical 
results.  Some format conversion operations produce results in a 
different radix than the operands."

according to the quietening requirement [2] set by the same standard:

"7.2 Invalid operation

"For operations producing results in floating-point format, the default 
result of an operation that signals the invalid operation exception 
shall be a quiet NaN that should provide some diagnostic information 
(see 6.2).

"These operations are:
    a)   any general-computational or signaling-computational operation 
         on a signaling NaN (see 6.2), except for some conversions (see 
         5.12)"

and the reference above is [3]:

"5.12 Details of conversion between floating-point data and external 
character sequences"

so does not apply to conversions a pair of floating-point formats.

Therefore quieten any sNaN encountered in floating-point format 
conversions, in the usual manner.

References:

[1] "IEEE Standard for Floating-Point Arithmetic", IEEE Computer 
    Society, IEEE Std 754-2008, 29 August 2008, pp. 21-22

[2] same, p. 37

[3] same, p. 30

Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---
 This is in particular how MIPS hardware operates, other processors 
supposedly do the same if they claim compliance to IEEE 754.

 Please apply.

qemu-softfloat-convert-snan.diff

Comments

Leon Alrae Jan. 29, 2015, 2:51 p.m. UTC | #1
On 09/12/2014 01:54, Maciej W. Rozycki wrote:
> Fix sNaN handling in floating-point format conversion operations, that 
> are classified by the IEEE 754-2008 standard as general-computational 
> operations [1]:
> 
> "5.4 formatOf general-computational operations
> 
> "5.4.2 Conversion operations for floating-point formats and decimal 
> character sequences
> 
> "Implementations shall provide the following formatOf conversion 
> operations from all supported floating-point formats to all supported 
> floating-point formats, as well as conversions to and from decimal 
> character sequences.  These operations shall not propagate non-canonical 
> results.  Some format conversion operations produce results in a 
> different radix than the operands."
> 
> according to the quietening requirement [2] set by the same standard:
> 
> "7.2 Invalid operation
> 
> "For operations producing results in floating-point format, the default 
> result of an operation that signals the invalid operation exception 
> shall be a quiet NaN that should provide some diagnostic information 
> (see 6.2).
> 
> "These operations are:
>     a)   any general-computational or signaling-computational operation 
>          on a signaling NaN (see 6.2), except for some conversions (see 
>          5.12)"
> 
> and the reference above is [3]:
> 
> "5.12 Details of conversion between floating-point data and external 
> character sequences"
> 
> so does not apply to conversions a pair of floating-point formats.
> 
> Therefore quieten any sNaN encountered in floating-point format 
> conversions, in the usual manner.
> 
> References:
> 
> [1] "IEEE Standard for Floating-Point Arithmetic", IEEE Computer 
>     Society, IEEE Std 754-2008, 29 August 2008, pp. 21-22
> 
> [2] same, p. 37
> 
> [3] same, p. 30
> 
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
>  This is in particular how MIPS hardware operates, other processors 
> supposedly do the same if they claim compliance to IEEE 754.
> 
>  Please apply.
> 
> qemu-softfloat-convert-snan.diff
> Index: qemu-git-trunk/fpu/softfloat.c

Reviewed-by: Leon Alrae <leon.alrae@imgtec.com>
Peter Maydell Feb. 5, 2015, 4:37 p.m. UTC | #2
On 9 December 2014 at 01:54, Maciej W. Rozycki <macro@codesourcery.com> wrote:
> Fix sNaN handling in floating-point format conversion operations, that
> are classified by the IEEE 754-2008 standard as general-computational
> operations [1]:
>
> "5.4 formatOf general-computational operations
>
> "5.4.2 Conversion operations for floating-point formats and decimal
> character sequences
>
> "Implementations shall provide the following formatOf conversion
> operations from all supported floating-point formats to all supported
> floating-point formats, as well as conversions to and from decimal
> character sequences.  These operations shall not propagate non-canonical
> results.  Some format conversion operations produce results in a
> different radix than the operands."
>
> according to the quietening requirement [2] set by the same standard:
>
> "7.2 Invalid operation
>
> "For operations producing results in floating-point format, the default
> result of an operation that signals the invalid operation exception
> shall be a quiet NaN that should provide some diagnostic information
> (see 6.2).
>
> "These operations are:
>     a)   any general-computational or signaling-computational operation
>          on a signaling NaN (see 6.2), except for some conversions (see
>          5.12)"
>
> and the reference above is [3]:
>
> "5.12 Details of conversion between floating-point data and external
> character sequences"
>
> so does not apply to conversions a pair of floating-point formats.
>
> Therefore quieten any sNaN encountered in floating-point format
> conversions, in the usual manner.
>
> References:
>
> [1] "IEEE Standard for Floating-Point Arithmetic", IEEE Computer
>     Society, IEEE Std 754-2008, 29 August 2008, pp. 21-22
>
> [2] same, p. 37
>
> [3] same, p. 30
>
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
>  This is in particular how MIPS hardware operates, other processors
> supposedly do the same if they claim compliance to IEEE 754.

ARM currently handles this issue by calling the _maybe_silence_nan()
function in all target-specific helper functions that call
float-to-float conversion functions (but that has its own
issues, see below).

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).

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.

thanks
-- PMM
Peter Maydell Feb. 5, 2015, 4:38 p.m. UTC | #3
Oops, forgot to fix up Maciej's email address...

On 5 February 2015 at 16:37, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 December 2014 at 01:54, Maciej W. Rozycki <macro@codesourcery.com> wrote:
>> Fix sNaN handling in floating-point format conversion operations, that
>> are classified by the IEEE 754-2008 standard as general-computational
>> operations [1]:

>>  This is in particular how MIPS hardware operates, other processors
>> supposedly do the same if they claim compliance to IEEE 754.
>
> ARM currently handles this issue by calling the _maybe_silence_nan()
> function in all target-specific helper functions that call
> float-to-float conversion functions (but that has its own
> issues, see below).
>
> 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).
>
> 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.
>
> thanks
> -- PMM
Maciej W. Rozycki Feb. 6, 2015, 2:37 p.m. UTC | #4
On Thu, 5 Feb 2015, Peter Maydell wrote:

> > Fix sNaN handling in floating-point format conversion operations, that
> > are classified by the IEEE 754-2008 standard as general-computational
> > operations [1]:
> >
> > "5.4 formatOf general-computational operations
> >
> > "5.4.2 Conversion operations for floating-point formats and decimal
> > character sequences
> >
> > "Implementations shall provide the following formatOf conversion
> > operations from all supported floating-point formats to all supported
> > floating-point formats, as well as conversions to and from decimal
> > character sequences.  These operations shall not propagate non-canonical
> > results.  Some format conversion operations produce results in a
> > different radix than the operands."
> >
> > according to the quietening requirement [2] set by the same standard:
> >
> > "7.2 Invalid operation
> >
> > "For operations producing results in floating-point format, the default
> > result of an operation that signals the invalid operation exception
> > shall be a quiet NaN that should provide some diagnostic information
> > (see 6.2).
> >
> > "These operations are:
> >     a)   any general-computational or signaling-computational operation
> >          on a signaling NaN (see 6.2), except for some conversions (see
> >          5.12)"
> >
> > and the reference above is [3]:
> >
> > "5.12 Details of conversion between floating-point data and external
> > character sequences"
> >
> > so does not apply to conversions a pair of floating-point formats.
> >
> > Therefore quieten any sNaN encountered in floating-point format
> > conversions, in the usual manner.
> >
> > References:
> >
> > [1] "IEEE Standard for Floating-Point Arithmetic", IEEE Computer
> >     Society, IEEE Std 754-2008, 29 August 2008, pp. 21-22
> >
> > [2] same, p. 37
> >
> > [3] same, p. 30
> >
> > Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> > ---
> >  This is in particular how MIPS hardware operates, other processors
> > supposedly do the same if they claim compliance to IEEE 754.
> 
> ARM currently handles this issue by calling the _maybe_silence_nan()
> function in all target-specific helper functions that call
> float-to-float conversion functions (but that has its own
> issues, see below).
> 
> 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.

 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.

 For one the x87 doesn't really implement proper single or double 
arithmetic.  While there are two precision control bits in its control 
register, not all arithmetic operations respect them, making their 
calculation unconditionally in the extended precision instead.  And even 
these that do only respect them for the significand and not the 
exponent[1]:

"
* The precision control (PC) bits (bits 9-8) can be used to set the MCP 
  internal operating precision of the significand at less than the default 
  of 64 bits (extended precision).  This can be useful in providing 
  compatibility with early generation arithmetic processors of smaller 
  precision.  PC affects only the instructions ADD, SUB, DIV, MUL, and 
  SQRT. For all other instructions, either the precision is determined by 
  the opcode or extended precision is used.
"

 So I think that any x87 intricacies are best handled either by hooks 
similar to `float64_convert_silence_nan' I propose or by individual 
implementation of whole operations where the actions required diverge from 
the standard so much as to make the use of such hooks infeasible.

 Besides are you sure?  I see x87 documentation say this[2]:

"
          |                                        |     Default Action
Exception |                Cause                   | (if exception is masked)
----------+----------------------------------------+-------------------------
Invalid   | Operation on a signaling NaN,          | Result is a quiet NaN,
Operation | unsupported format, indeterminate form | integer indeterminate,
          | (0*Inf, 0/0, (+Inf)+(-Inf), etc.), or  | or BCD indefinite
          | stack overflow/underflow (SF is also   |
          | set).                                  |
"

and this[3]:

"
11. FLD single/double precision when the operand is denormal converts the 
    number to extended precision and signals the denormalized operand 
    exception.  When loading a signaling NaN, FLD single/double precision 
    signals an invalid-operand exception.
"

so it looks to me conversions do actually quieten an sNaN.  Or are you 
referring to earlier x87 implementations that did not implement sNaNs[3]:

"
12. The Intel387 DX MCP only generates quiet NaNs (as on the 80287); 
    however, the Intel387 DX MCP distinguishes between quiet NaNs and 
    signaling NaNs.  Signaling NaNs trigger exceptions when they are used 
    as operands; quiet NaNs do not (except for FCOM, FIST, and FBSTP which 
    also raise IE for quiet NaNs).
"

But does QEMU support 16-bit x86 machines?  Or at least a configuration 
where an 80386 CPU is paired with an 80287 FPU -- which is a bit unusual, 
but architecturally supported (see the CR0.ET bit, in later 80386 versions 
as well as all newer architecture revisions removed and the bit location 
hardwired to 1 instead as for an 80386 CPU paired with an 80387 FPU)?

 If so, then even more variance will have to be taken into account here as 
far as x87 is considered, and I believe it all can be handled by using 
such `float*_convert_silence_nan' hooks proposed.

> 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.  Of course someone will have to implement it and 
it's not entirely clear to me at the moment who that person would be.

 References:

[1] "Intel387 DX Math CoProcessor", Intel Corporation, Order Number:
    240448-005, March 1992, Section 2.3 "Register Set", Subsection 2.3.5 
    "Control Word", p. 14

[2] same, Table 2.7 "Exceptions", p. 16

[3] same, Section 2.7 "8087 and 80287 Compatibility", Subsection 2.7.2 
    "Exceptions", p. 17

  Maciej
Peter Maydell Feb. 6, 2015, 2:45 p.m. UTC | #5
On 6 February 2015 at 14:37, Maciej W. Rozycki <macro@linux-mips.org> 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
Maciej W. Rozycki Feb. 6, 2015, 7:35 p.m. UTC | #6
On Fri, 6 Feb 2015, Peter Maydell wrote:

> >  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.

 Somehow I missed from your description that ARM actually requires actions 
at an earlier stage, sorry.  What I understood is that it is only the sign 
bit that is lost, and that could be easily handled with a bespoke version 
of `float64_convert_silence_nan', etc.  Is that the sign bit is supposed 
to be preserved in quietening an sNaN iff truncation of a non-zero wider 
trailing significand field produced all-zeros in the narrower result, and 
otherwise set to zero then?

> >  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...

 I agree making sure no target has regressed is a valuable goal, but TBH 
I'd welcome some cooperation from target maintainers here, especially as 
the current situation looks like core code is non-compliant and I find it
hard to believe all the processors out there (except MIPS) got their IEEE 
754 implementation wrong.  So what this change does I'd expect actually to 
*progress* most of the targets, except from maybe a couple of oddballs.

 And I would very much appreciate feedback from target maintainers of 
these oddball architectures, after all they know the internals of the 
processors they care of the best!

> >  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.

 We can surely sort out the level of support x87 requires in further 
updates to the proposed change.  I'd expect x86's more modern SSE 
implementation of IEEE 754, that I'm less familiar with (which I guess 
just shows my age), got the details just as the standard expects them, 
which in turn implies more complication on our side.  But if we set the 
minimum at 80387, then as it stands it does not appear to me that this 
change will regress anything.

 But as I say, I think it would be most productive if target maintainers 
of at least ARM and x86, and maybe other architectures as well if they 
have quirks in this area, chimed in and identified issues.  I think it is 
impractical to require submitters of a generic change like this to chase 
architecture manuals of all the processors affected and study and 
understand their instruction sets to infer whether the change is going to 
have a negative effect.  Especially given that from the current code and 
the standard it is supposed to implement it is clear to me that the code 
is buggy.

 I also think that papering the issue over by fixing the thing up somehow 
in the MIPS TCG and letting other people worry about their architecture of 
interest is not the way to go, although it clearly looks like the path of 
least resistance to me.

> >> 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?

  Maciej
Maciej W. Rozycki Feb. 8, 2015, 12:12 p.m. UTC | #7
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
diff mbox

Patch

Index: qemu-git-trunk/fpu/softfloat.c
===================================================================
--- qemu-git-trunk.orig/fpu/softfloat.c	2014-10-24 22:03:48.000000000 +0100
+++ qemu-git-trunk/fpu/softfloat.c	2014-11-03 01:45:15.488923310 +0000
@@ -1711,7 +1711,11 @@  float64 float32_to_float64( float32 a ST
     aExp = extractFloat32Exp( a );
     aSign = extractFloat32Sign( a );
     if ( aExp == 0xFF ) {
-        if ( aSig ) return commonNaNToFloat64( float32ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig) {
+            return float64_maybe_silence_nan(
+                commonNaNToFloat64(float32ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
+        }
         return packFloat64( aSign, 0x7FF, 0 );
     }
     if ( aExp == 0 ) {
@@ -1741,7 +1745,11 @@  floatx80 float32_to_floatx80( float32 a 
     aExp = extractFloat32Exp( a );
     aSign = extractFloat32Sign( a );
     if ( aExp == 0xFF ) {
-        if ( aSig ) return commonNaNToFloatx80( float32ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig) {
+            return floatx80_maybe_silence_nan(
+                commonNaNToFloatx80(float32ToCommonNaN(a STATUS_VAR)
+                                    STATUS_VAR));
+        }
         return packFloatx80( aSign, 0x7FFF, LIT64( 0x8000000000000000 ) );
     }
     if ( aExp == 0 ) {
@@ -1771,7 +1779,11 @@  float128 float32_to_float128( float32 a 
     aExp = extractFloat32Exp( a );
     aSign = extractFloat32Sign( a );
     if ( aExp == 0xFF ) {
-        if ( aSig ) return commonNaNToFloat128( float32ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig) {
+            return float128_maybe_silence_nan(
+                commonNaNToFloat128(float32ToCommonNaN(a STATUS_VAR)
+                                    STATUS_VAR));
+        }
         return packFloat128( aSign, 0x7FFF, 0, 0 );
     }
     if ( aExp == 0 ) {
@@ -3151,7 +3163,11 @@  float32 float64_to_float32( float64 a ST
     aExp = extractFloat64Exp( a );
     aSign = extractFloat64Sign( a );
     if ( aExp == 0x7FF ) {
-        if ( aSig ) return commonNaNToFloat32( float64ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig) {
+            return float32_maybe_silence_nan(
+                commonNaNToFloat32(float64ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
+        }
         return packFloat32( aSign, 0xFF, 0 );
     }
     shift64RightJamming( aSig, 22, &aSig );
@@ -3318,7 +3334,9 @@  float32 float16_to_float32(float16 a, fl
 
     if (aExp == 0x1f && ieee) {
         if (aSig) {
-            return commonNaNToFloat32(float16ToCommonNaN(a STATUS_VAR) STATUS_VAR);
+            return float32_maybe_silence_nan(
+                commonNaNToFloat32(float16ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         return packFloat32(aSign, 0xff, 0);
     }
@@ -3351,8 +3369,9 @@  float16 float32_to_float16(float32 a, fl
                 float_raise(float_flag_invalid STATUS_VAR);
                 return packFloat16(aSign, 0, 0);
             }
-            return commonNaNToFloat16(
-                float32ToCommonNaN(a STATUS_VAR) STATUS_VAR);
+            return float16_maybe_silence_nan(
+                commonNaNToFloat16(float32ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         /* Infinity */
         if (!ieee) {
@@ -3389,8 +3408,9 @@  float64 float16_to_float64(float16 a, fl
 
     if (aExp == 0x1f && ieee) {
         if (aSig) {
-            return commonNaNToFloat64(
-                float16ToCommonNaN(a STATUS_VAR) STATUS_VAR);
+            return float64_maybe_silence_nan(
+                commonNaNToFloat64(float16ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         return packFloat64(aSign, 0x7ff, 0);
     }
@@ -3424,8 +3444,9 @@  float16 float64_to_float16(float64 a, fl
                 float_raise(float_flag_invalid STATUS_VAR);
                 return packFloat16(aSign, 0, 0);
             }
-            return commonNaNToFloat16(
-                float64ToCommonNaN(a STATUS_VAR) STATUS_VAR);
+            return float16_maybe_silence_nan(
+                commonNaNToFloat16(float64ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         /* Infinity */
         if (!ieee) {
@@ -3470,7 +3491,11 @@  floatx80 float64_to_floatx80( float64 a 
     aExp = extractFloat64Exp( a );
     aSign = extractFloat64Sign( a );
     if ( aExp == 0x7FF ) {
-        if ( aSig ) return commonNaNToFloatx80( float64ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig) {
+            return floatx80_maybe_silence_nan(
+                commonNaNToFloatx80(float64ToCommonNaN(a STATUS_VAR)
+                                    STATUS_VAR));
+        }
         return packFloatx80( aSign, 0x7FFF, LIT64( 0x8000000000000000 ) );
     }
     if ( aExp == 0 ) {
@@ -3501,7 +3526,11 @@  float128 float64_to_float128( float64 a 
     aExp = extractFloat64Exp( a );
     aSign = extractFloat64Sign( a );
     if ( aExp == 0x7FF ) {
-        if ( aSig ) return commonNaNToFloat128( float64ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig) {
+            return float128_maybe_silence_nan(
+                commonNaNToFloat128(float64ToCommonNaN(a STATUS_VAR)
+                                    STATUS_VAR));
+        }
         return packFloat128( aSign, 0x7FFF, 0, 0 );
     }
     if ( aExp == 0 ) {
@@ -4759,8 +4788,10 @@  float32 floatx80_to_float32( floatx80 a 
     aExp = extractFloatx80Exp( a );
     aSign = extractFloatx80Sign( a );
     if ( aExp == 0x7FFF ) {
-        if ( (uint64_t) ( aSig<<1 ) ) {
-            return commonNaNToFloat32( floatx80ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if ((uint64_t)(aSig << 1)) {
+            return float32_maybe_silence_nan(
+                commonNaNToFloat32(floatx80ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         return packFloat32( aSign, 0xFF, 0 );
     }
@@ -4787,8 +4818,10 @@  float64 floatx80_to_float64( floatx80 a 
     aExp = extractFloatx80Exp( a );
     aSign = extractFloatx80Sign( a );
     if ( aExp == 0x7FFF ) {
-        if ( (uint64_t) ( aSig<<1 ) ) {
-            return commonNaNToFloat64( floatx80ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if ((uint64_t)(aSig << 1)) {
+            return float64_maybe_silence_nan(
+                commonNaNToFloat64(floatx80ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         return packFloat64( aSign, 0x7FF, 0 );
     }
@@ -4814,8 +4847,9 @@  float128 floatx80_to_float128( floatx80 
     aSig = extractFloatx80Frac( a );
     aExp = extractFloatx80Exp( a );
     aSign = extractFloatx80Sign( a );
-    if ( ( aExp == 0x7FFF ) && (uint64_t) ( aSig<<1 ) ) {
-        return commonNaNToFloat128( floatx80ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+    if (aExp == 0x7FFF && (uint64_t)(aSig << 1)) {
+        return float128_maybe_silence_nan(
+            commonNaNToFloat128(floatx80ToCommonNaN(a STATUS_VAR) STATUS_VAR));
     }
     shift128Right( aSig<<1, 0, 16, &zSig0, &zSig1 );
     return packFloat128( aSign, aExp, zSig0, zSig1 );
@@ -5832,8 +5866,10 @@  float32 float128_to_float32( float128 a 
     aExp = extractFloat128Exp( a );
     aSign = extractFloat128Sign( a );
     if ( aExp == 0x7FFF ) {
-        if ( aSig0 | aSig1 ) {
-            return commonNaNToFloat32( float128ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig0 | aSig1) {
+            return float32_maybe_silence_nan(
+                commonNaNToFloat32(float128ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         return packFloat32( aSign, 0xFF, 0 );
     }
@@ -5866,8 +5902,10 @@  float64 float128_to_float64( float128 a 
     aExp = extractFloat128Exp( a );
     aSign = extractFloat128Sign( a );
     if ( aExp == 0x7FFF ) {
-        if ( aSig0 | aSig1 ) {
-            return commonNaNToFloat64( float128ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig0 | aSig1) {
+            return float64_maybe_silence_nan(
+                commonNaNToFloat64(float128ToCommonNaN(a STATUS_VAR)
+                                   STATUS_VAR));
         }
         return packFloat64( aSign, 0x7FF, 0 );
     }
@@ -5899,8 +5937,10 @@  floatx80 float128_to_floatx80( float128 
     aExp = extractFloat128Exp( a );
     aSign = extractFloat128Sign( a );
     if ( aExp == 0x7FFF ) {
-        if ( aSig0 | aSig1 ) {
-            return commonNaNToFloatx80( float128ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
+        if (aSig0 | aSig1) {
+            return floatx80_maybe_silence_nan(
+                commonNaNToFloatx80(float128ToCommonNaN(a STATUS_VAR)
+                                    STATUS_VAR));
         }
         return packFloatx80( aSign, 0x7FFF, LIT64( 0x8000000000000000 ) );
     }