Re: [Qemu-devel] [PATCH v1 10/19] fpu/softfloat: re-factor add/sub

[Richard Henderson <richard.henderson@linaro.org>]

On 12/11/2017 04:56 AM, Alex Bennée wrote:
> We can now add float16_add/sub and use the common decompose and
> canonicalize functions to have a single implementation for
> float16/32/64 add and sub functions.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

I was involved in writing this, so

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

However,

> +/*
> + * Returns the result of adding the absolute values of the
> + * floating-point values `a' and `b'. If `subtract' is set, the sum is
> + * negated before being returned. `subtract' is ignored if the result
> + * is a NaN. The addition is performed according to the IEC/IEEE
> + * Standard for Binary Floating-Point Arithmetic.
> + */
> +
> +static decomposed_parts add_decomposed(decomposed_parts a, decomposed_parts b,
> +                                       bool subtract, float_status *s)

The comment does not accurately describe what the function does, particularly
wrt subtract.

> +        if (a.cls >= float_class_qnan
> +            ||
> +            b.cls >= float_class_qnan)

Would you please fix this up throughout the patch set?
While I prefer the GNU

  (X
   || Y)

I'm also ok with

  (X ||
   Y)

but || on a line by itself is just weird.


r~