qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH] fpu: Simplify floatx80ToCommonNaN function.


From: Paolo Bonzini
Subject: Re: [Qemu-trivial] [PATCH] fpu: Simplify floatx80ToCommonNaN function.
Date: Fri, 31 May 2013 14:24:25 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6

Il 31/05/2013 14:07, Michael Tokarev ha scritto:
> 31.05.2013 13:39, Thomas Schwinge wrote:
>> Signed-off-by: Thomas Schwinge <address@hidden>
>> ---
>>  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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]