Re: soft-fp: Refactor exception handling for comparisons

["Carlos O'Donell" <carlos at redhat dot com>]

On 10/08/2014 08:31 PM, Joseph S. Myers wrote:
> On Wed, 8 Oct 2014, Carlos O'Donell wrote:
> 
>>> +/* Helper for comparisons.  EX is 0 not to raise exceptions, 1 to
>>> +   raise exceptions for signaling NaN operands, 2 to raise exceptions
>>> +   for all NaN operands.  */
>>> +
>>> +#define _FP_CMP_CHECK_NAN(fs, wc, X, Y, ex)		\
>>> +  do							\
>>> +    {							\
>>> +      if (ex)						\
>>
>> Does the soft-fp code care about implicit boolean coersion?
> 
> Not based on existing handling of rsigned in _FP_TO_INT.  I've made a note 
> to review that as a coding style issue (I have a few other residual coding 
> style issues in this code to deal with, although lots have been fixed).

Thanks.

>> If I had to write this it would have written:
>>
>> if ((ex) == 1 
>>     && (_FP_ISSIGNAN (fs, wc, X) 
>>         || _FP_ISSIGNAN (fs, wc, Y))
>>   FP_SET_EXCEPTION (FP_EX_INVALID);
>>
>> if ((ex) == 2)
>>   FP_SET_EXCEPTION (FP_EX_INVALID);
>>
>> Since this is *very* easy to read and understand given the description,
>> but I don't know if the compiler generates better or worse code given
>> the above.
> 
> The code is written to make it obvious that things can be folded for 
> constant arguments to the macros, rather than based on what code a 
> particular compiler version does or does not generate.  (If it's 
> convenient to do things in a way that doesn't change compiler code 
> generation at all, so making it obvious that a patch is safe for existing 
> users not using the new features, that's a bonus, though it wasn't 
> possible for this patch.)

Oh well. I guess my objection to your even further complication to this
macro does not stand.

Would you consider adding a comment stating why the macro is written
as such?

Cheers,
Carlos.