Re: [Qemu-devel] [PATCH v2] softfloat: float32_to_float16() should do inexact instead of underflow for rounding case

[Peter Maydell]

On 3 May 2012 15:37, Alexey Starikovskiy <address@hidden> wrote:
> IEEE Standard for Floating-Point Arithmetic:
> 7.5 Underflow
> The underflow exception shall be signaled when a tiny non-zero result
> is detected. For binary formats, this
> shall be either:
> a) after rounding — when a non-zero result computed as though the
> exponent range were unbounded
> would lie strictly between ± b emin, or
> b) before rounding — when a non-zero result computed as though both
> the exponent range and the
> precision were unbounded would lie strictly between ± b emin.
>
> The implementer shall choose how tininess is detected, but shall
> detect tininess in the same way for all
> operations in radix two, including conversion operations under a
> binary rounding attribute.
>
> ....
>
> In addition, under default exception handling for underflow, if the
> rounded result is inexact — that is, it
> differs from what would have been computed were both exponent range
> and precision unbounded — the
> underflow flag shall be raised and the inexact (see 7.6) exception
> shall be signaled. If the rounded result is
> exact, no flag is raised and no inexact exception is signaled.
>
> ....
>
> 7.6 Inexact
> Unless stated otherwise, if the rounded result of an operation is
> inexact — that is, it differs from what would
> have been computed were both exponent range and precision unbounded —
> then the inexact exception shall
> be signaled. The rounded or overflowed result shall be delivered to
> the destination.
>
> Signed-off-by: Alexey Starikovskiy <address@hidden>
> ---
>  fpu/softfloat.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 9e1b5f9..d4a963f 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -3067,7 +3067,11 @@ float16 float32_to_float16(float32 a, flag ieee
> STATUS_PARAM)
>         mask = 0x00001fff;
>     }
>     if (aSig & mask) {
> -        float_raise( float_flag_underflow STATUS_VAR );
> +        if (aExp < -14 &&
> +            STATUS(float_detect_tininess) == float_tininess_before_rounding) 
> {
> +            float_raise( float_flag_underflow STATUS_VAR);
> +        }
> +        float_raise( float_flag_inexact STATUS_VAR );t
>         roundingMode = STATUS(float_rounding_mode);
>         switch (roundingMode) {
>         case float_round_nearest_even:
> @@ -3091,9 +3095,6 @@ float16 float32_to_float16(float32 a, flag ieee
> STATUS_PARAM)
>             aSig >>= 1;
>             aExp++;
>         }
> -    } else if (aExp < -14
> -          && STATUS(float_detect_tininess) == 
> float_tininess_before_rounding) {
> -        float_raise( float_flag_underflow STATUS_VAR);
>     }
>
>     if (ieee) {

I'm afraid this patch doesn't apply, because your mail client
has mangled it. Specifically:
 * it's multipart/mixed because you attached the test case to
the patch email
 * long lines have been wrapped, which breaks the patch format

It also has unnecessary non-ASCII chars in the commit message.

Your commit message quotes way too much of the standard and
doesn't provide sufficient rationale and information about
what you're actually changing and why. The initial summary
(subject) line is also now out of date as you haven't
changed it between v1 and v2.

I think we also have the tininess-after-rounding case wrong,
though nobody uses that at the moment.

Both the existing code and the code after your patch will
incorrectly raise the inexact exception for the case of
non-IEEE halfprec format where the result overflows -- this
should set InvalidOp and not Inexact.

Other than that it looks like a good fix.

I would also suggest losing the spaces around the arguments
to float_raise() -- I know the code is inconsistent about this
since it's a combination of a weirdo old style from softfloat
plus newer code using qemu style.

-- PMM