Re: [Qemu-devel] [PATCH v3 04/13] fpu: use min/max values from stdint.h for integral overflow

["Alex Bennée" <alex.bennee@linaro.org>]

Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:

> 13.08.2019. 14.52, "Alex Bennée" <alex.bennee@linaro.org> је написао/ла:
>>
>> Remove some more use of LIT64 while making the meaning more clear. We
>> also avoid the need of casts as the results by definition fit into the
>> return type.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  fpu/softfloat.c | 30 ++++++++++++++----------------
>>  1 file changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 9e57b7b5933..a1e1e9a8559 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -3444,9 +3444,7 @@ static int64_t roundAndPackInt64(flag zSign,
> uint64_t absZ0, uint64_t absZ1,
>>      if ( z && ( ( z < 0 ) ^ zSign ) ) {
>>   overflow:
>>          float_raise(float_flag_invalid, status);
>> -        return
>> -              zSign ? (int64_t) LIT64( 0x8000000000000000 )
>> -            : LIT64( 0x7FFFFFFFFFFFFFFF );
>> +        return zSign ? INT64_MIN : INT64_MAX;
>>      }
>
> In function roundAndPavkInt32 tgere is a following segment:
>
>     if ( ( absZ>>32 ) || ( z && ( ( z < 0 ) ^ zSign ) ) ) {
>         float_raise(float_flag_invalid, status);
>         return zSign ? (int32_t) 0x80000000 : 0x7FFFFFFF;
>     }
>
> Perhaps replace these constants with INT32_MIN, INT32_MAX, for similar
> reasons, in the same or a separate patch?

Yeah that was missed seeing as I picked up one of the INT32 cases later on.

>
> Aleksandar
>
>
>>      if (absZ1) {
>>          status->float_exception_flags |= float_flag_inexact;
>> @@ -3497,7 +3495,7 @@ static int64_t roundAndPackUint64(flag zSign,
> uint64_t absZ0,
>>          ++absZ0;
>>          if (absZ0 == 0) {
>>              float_raise(float_flag_invalid, status);
>> -            return LIT64(0xFFFFFFFFFFFFFFFF);
>> +            return UINT64_MAX;
>>          }
>>          absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
>>      }
>> @@ -5518,9 +5516,9 @@ int64_t floatx80_to_int64(floatx80 a, float_status
> *status)
>>          if ( shiftCount ) {
>>              float_raise(float_flag_invalid, status);
>>              if (!aSign || floatx80_is_any_nan(a)) {
>> -                return LIT64( 0x7FFFFFFFFFFFFFFF );
>> +                return INT64_MAX;
>>              }
>> -            return (int64_t) LIT64( 0x8000000000000000 );
>> +            return INT64_MIN;
>>          }
>>          aSigExtra = 0;
>>      }
>> @@ -5561,10 +5559,10 @@ int64_t floatx80_to_int64_round_to_zero(floatx80
> a, float_status *status)
>>          if ( ( a.high != 0xC03E ) || aSig ) {
>>              float_raise(float_flag_invalid, status);
>>              if ( ! aSign || ( ( aExp == 0x7FFF ) && aSig ) ) {
>> -                return LIT64( 0x7FFFFFFFFFFFFFFF );
>> +                return INT64_MAX;
>>              }
>>          }
>> -        return (int64_t) LIT64( 0x8000000000000000 );
>> +        return INT64_MIN;
>>      }
>>      else if ( aExp < 0x3FFF ) {
>>          if (aExp | aSig) {
>> @@ -6623,7 +6621,7 @@ int32_t float128_to_int32_round_to_zero(float128 a,
> float_status *status)
>>      if ( ( z < 0 ) ^ aSign ) {
>>   invalid:
>>          float_raise(float_flag_invalid, status);
>> -        return aSign ? (int32_t) 0x80000000 : 0x7FFFFFFF;
>> +        return aSign ? INT32_MIN : INT32_MAX;
>>      }
>>      if ( ( aSig0<<shiftCount ) != savedASig ) {
>>          status->float_exception_flags |= float_flag_inexact;
>> @@ -6662,9 +6660,9 @@ int64_t float128_to_int64(float128 a, float_status
> *status)
>>                        && ( aSig1 || ( aSig0 != LIT64( 0x0001000000000000
> ) ) )
>>                      )
>>                 ) {
>> -                return LIT64( 0x7FFFFFFFFFFFFFFF );
>> +                return INT64_MAX;
>>              }
>> -            return (int64_t) LIT64( 0x8000000000000000 );
>> +            return INT64_MIN;
>>          }
>>          shortShift128Left( aSig0, aSig1, - shiftCount, &aSig0, &aSig1 );
>>      }
>> @@ -6710,10 +6708,10 @@ int64_t float128_to_int64_round_to_zero(float128
> a, float_status *status)
>>              else {
>>                  float_raise(float_flag_invalid, status);
>>                  if ( ! aSign || ( ( aExp == 0x7FFF ) && ( aSig0 | aSig1
> ) ) ) {
>> -                    return LIT64( 0x7FFFFFFFFFFFFFFF );
>> +                    return INT64_MAX;
>>                  }
>>              }
>> -            return (int64_t) LIT64( 0x8000000000000000 );
>> +            return INT64_MIN;
>>          }
>>          z = ( aSig0<<shiftCount ) | ( aSig1>>( ( - shiftCount ) & 63 ) );
>>          if ( (uint64_t) ( aSig1<<shiftCount ) ) {
>> @@ -6764,19 +6762,19 @@ uint64_t float128_to_uint64(float128 a,
> float_status *status)
>>      if (aSign && (aExp > 0x3FFE)) {
>>          float_raise(float_flag_invalid, status);
>>          if (float128_is_any_nan(a)) {
>> -            return LIT64(0xFFFFFFFFFFFFFFFF);
>> +            return UINT64_MAX;
>>          } else {
>>              return 0;
>>          }
>>      }
>>      if (aExp) {
>> -        aSig0 |= LIT64(0x0001000000000000);
>> +        aSig0 |= UINT64_C(0x0001000000000000);
>>      }
>>      shiftCount = 0x402F - aExp;
>>      if (shiftCount <= 0) {
>>          if (0x403E < aExp) {
>>              float_raise(float_flag_invalid, status);
>> -            return LIT64(0xFFFFFFFFFFFFFFFF);
>> +            return UINT64_MAX;
>>          }
>>          shortShift128Left(aSig0, aSig1, -shiftCount, &aSig0, &aSig1);
>>      } else {
>> --
>> 2.20.1
>>
>>


--
Alex Bennée