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

Alex Bennée posted 13 patches 6 years, 6 months ago
Maintainers: Aleksandar Markovic <amarkovic@wavecomp.com>, Aleksandar Rikalo <arikalo@wavecomp.com>, Aurelien Jarno <aurelien@aurel32.net>
[Qemu-devel] [PATCH v3 04/13] fpu: use min/max values from stdint.h for integral overflow
Posted by Alex Bennée 6 years, 6 months ago
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;
     }
     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


Re: [Qemu-devel] [PATCH v3 04/13] fpu: use min/max values from stdint.h for integral overflow
Posted by Richard Henderson 6 years, 6 months ago
On 8/13/19 1:49 PM, Alex Bennée wrote:
> 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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


Re: [Qemu-devel] [PATCH v3 04/13] fpu: use min/max values from stdint.h for integral overflow
Posted by Aleksandar Markovic 6 years, 5 months ago
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?

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
>
>
Re: [Qemu-devel] [PATCH v3 04/13] fpu: use min/max values from stdint.h for integral overflow
Posted by Alex Bennée 6 years, 5 months ago
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