I explicitly left the check off the comparison operations because I misread the NaN check as something equivalent to the check I would be adding. I'll add it shortly.
With regards to adding int32_indefinite, etc constants, I think I'll leave it as is -- I'd prefer to have *what* happens clear (return this number), then have *why* it happens be clear (return the integer indefinite value). And, finally, yes, I know what C++ and C-style comments are :P I just think every argument I've ever read in favor of only using the latter has been complete nonsense. Regardless! Guidelines are guidelines. Thanks, - Andrew On Tue, Aug 16, 2016 at 6:34 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > 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
