Il 31/05/2013 14:07, Michael Tokarev ha scritto:
> 31.05.2013 13:39, Thomas Schwinge wrote:
>> Signed-off-by: Thomas Schwinge <tho...@codesourcery.com>
>> ---
>> fpu/softfloat-specialize.h | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git fpu/softfloat-specialize.h fpu/softfloat-specialize.h
>> index 518f694..83add1a 100644
>> --- fpu/softfloat-specialize.h
>> +++ fpu/softfloat-specialize.h
>> @@ -934,15 +934,14 @@ static commonNaNT floatx80ToCommonNaN( floatx80 a
>> STATUS_PARAM)
>> commonNaNT z;
>>
>> if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid
>> STATUS_VAR);
>> - if ( a.low >> 63 ) {
>> - z.sign = a.high >> 15;
>> - z.low = 0;
>> - z.high = a.low << 1;
>> - } else {
>> - z.sign = floatx80_default_nan_high >> 15;
>> - z.low = 0;
>> - z.high = floatx80_default_nan_low << 1;
>> + /* Replace a Pseudo NaN with a default NaN. */
>> + if (!(a.low >> 63)) {
>> + a.low = floatx80_default_nan_low;
>> + a.high = floatx80_default_nan_high;
>> }
>> + z.sign = a.high >> 15;
>> + z.low = 0;
>> + z.high = a.low << 1;
>> return z;
>> }
>
> Hmm. And where's the simplification? Here's context diff for the same:
>
> *** fpu/softfloat-specialize.h.orig 2013-05-31 16:02:51.614710351 +0400
> --- fpu/softfloat-specialize.h 2013-05-31 16:02:59.838820308 +0400
> ***************
> *** 936,946 ****
> if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid
> STATUS_VAR);
> ! if ( a.low >> 63 ) {
> ! z.sign = a.high >> 15;
> ! z.low = 0;
> ! z.high = a.low << 1;
> ! } else {
> ! z.sign = floatx80_default_nan_high >> 15;
> ! z.low = 0;
> ! z.high = floatx80_default_nan_low << 1;
> }
> return z;
> --- 936,945 ----
> if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid
> STATUS_VAR);
> ! /* Replace a Pseudo NaN with a default NaN. */
> ! if (!(a.low >> 63)) {
> ! a.low = floatx80_default_nan_low;
> ! a.high = floatx80_default_nan_high;
> }
> + z.sign = a.high >> 15;
> + z.low = 0;
> + z.high = a.low << 1;
> return z;
>
>
> Yes, your version is 3 lines shorter, because it
> does not have extra else{} (2 lines) and the
> remaining if() construct is one line shorter too,
> due to moving z.low=0 construct into common place.
>
> But I don't think your version is more readable, --
> before it was easy to understand what is going on,
> we had two easy case with all right stuff done for
> each case. Now we do some preparation before, so
> the common case works.
>
> Generated code should be about the same anyway, but
> to me (IMHO!), original code is a bit more readable.
I agree. It's also not trivial.
Paolo
