On 15 August 2016 at 23:27, Andrew Dutcher <and...@andrewdutcher.com> 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 <and...@andrewdutcher.com> > --- > > 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