Re: [Qemu-devel] [PATCH v4] fpu: add mechanism to check for invalid long double formats

[Peter Maydell]

On 15 August 2016 at 23:27, Andrew Dutcher <address@hidden> wrote:
> All operations that take a floatx80 as an operand need to have their
> inputs checked for malformed encodings. In all of these cases, use the
> function floatx80_invalid_encoding to perform the check. If an invalid
> operand is found, raise an invalid operation exception, and then return
> either NaN (for fp-typed results) or the integer indefinite value (the
> minimum representable signed integer value, for int-typed results).
>
> Signed-off-by: Andrew Dutcher <address@hidden>
> ---
>
> Version 4: Remove comments, since apparently it's still 1998. If anyone wants
> to know what the value is for, they can check git blame.

The code style gripe is not for having comments, it's for
using the "//" style comment rather than "/* ... */". Yeah,
we have some odd style requirements; at least we have a script
which will detect them automatically. (By the way, you can run
./scripts/checkpatch.pl on your patch before sending it if you
like; you don't have to wait for the patch robot to read your
email :-))

If you liked you could  define a constant for the 32-bit and
64-bit indefinite values rather than using 1 << 31 &c directly;
'return int32_indefinite;' is sufficiently self-documenting
not to need a comment.

I don't mind whether you do that, or not; your choice.

The code changes you have here are good, but you've forgotten
one piece: the comparison ops also need to handle the invalid
encodings.

floatx80_compare_internal() needs to raise float_flag_invalid
and return float_relation_unordered if either of its inputs are
invalid encodings.

There are also separate single-comparison functions:
floatx80_eq(), floatx80_le(), floatx80_lt(), floatx80_unordered(),
floatx80_eq_quiet(), floatx80_le_quiet(), floatx80_lt_quiet(),
floatx80_unordered_quiet(). The i386 guest doesn't use them but
I think for consistency we should treat invalid encodings like
NaNs there: raise float_flag_invalid and return 0.

thanks
-- PMM