Also- I'm having issues applying the new patch: @@ -5768,7 +5774,9 @@ int floatx80_lt(floatx80 a, floatx80 b, float_status *status) *----------------------------------------------------------------------------*/ int floatx80_unordered(floatx80 a, floatx80 b, float_status *status) { - if ( ( ( extractFloatx80Exp( a ) == 0x7FFF ) + if ( floatx80_invalid_encoding( a ) + || floatx80_invalid_encoding( b ) + || ( ( extractFloatx80Exp( a ) == 0x7FFF ) && (uint64_t) ( extractFloatx80Frac( a )<<1 ) ) || ( ( extractFloatx80Exp( b ) == 0x7FFF ) && (uint64_t) ( extractFloatx80Frac( b )<<1 ) )
When I do this, the style checker complains about the spaces after the open parens and before the close parens. I now have to change this entire stanza to be styled correctly, since I'm replacing the original first line... On Tue, Aug 16, 2016 at 3:59 PM, Andrew Dutcher <and...@andrewdutcher.com> wrote: > 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