Re: [PATCH 04/72] accel/tcg: Use add/sub overflow routines in tcg-runtime-gvec.c

[David Hildenbrand <david@redhat.com>]

On 08.05.21 03:46, Richard Henderson wrote:
> Obvious uses of the new functions.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tcg-runtime-gvec.c | 36 ++++++++++++++++--------------------
>   1 file changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/accel/tcg/tcg-runtime-gvec.c b/accel/tcg/tcg-runtime-gvec.c
> index 521da4a813..ac7d28c251 100644
> --- a/accel/tcg/tcg-runtime-gvec.c
> +++ b/accel/tcg/tcg-runtime-gvec.c
> @@ -1073,9 +1073,8 @@ void HELPER(gvec_ssadd32)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(int32_t)) {
>           int32_t ai = *(int32_t *)(a + i);
>           int32_t bi = *(int32_t *)(b + i);
> -        int32_t di = ai + bi;
> -        if (((di ^ ai) &~ (ai ^ bi)) < 0) {
> -            /* Signed overflow.  */
> +        int32_t di;
> +        if (sadd32_overflow(ai, bi, &di)) {
>               di = (di < 0 ? INT32_MAX : INT32_MIN);
>           }
>           *(int32_t *)(d + i) = di;
> @@ -1091,9 +1090,8 @@ void HELPER(gvec_ssadd64)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(int64_t)) {
>           int64_t ai = *(int64_t *)(a + i);
>           int64_t bi = *(int64_t *)(b + i);
> -        int64_t di = ai + bi;
> -        if (((di ^ ai) &~ (ai ^ bi)) < 0) {
> -            /* Signed overflow.  */
> +        int64_t di;
> +        if (sadd64_overflow(ai, bi, &di)) {
>               di = (di < 0 ? INT64_MAX : INT64_MIN);
>           }
>           *(int64_t *)(d + i) = di;
> @@ -1143,9 +1141,8 @@ void HELPER(gvec_sssub32)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(int32_t)) {
>           int32_t ai = *(int32_t *)(a + i);
>           int32_t bi = *(int32_t *)(b + i);
> -        int32_t di = ai - bi;
> -        if (((di ^ ai) & (ai ^ bi)) < 0) {
> -            /* Signed overflow.  */
> +        int32_t di;
> +        if (ssub32_overflow(ai, bi, &di)) {
>               di = (di < 0 ? INT32_MAX : INT32_MIN);
>           }
>           *(int32_t *)(d + i) = di;
> @@ -1161,9 +1158,8 @@ void HELPER(gvec_sssub64)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(int64_t)) {
>           int64_t ai = *(int64_t *)(a + i);
>           int64_t bi = *(int64_t *)(b + i);
> -        int64_t di = ai - bi;
> -        if (((di ^ ai) & (ai ^ bi)) < 0) {
> -            /* Signed overflow.  */
> +        int64_t di;
> +        if (ssub64_overflow(ai, bi, &di)) {
>               di = (di < 0 ? INT64_MAX : INT64_MIN);
>           }
>           *(int64_t *)(d + i) = di;
> @@ -1209,8 +1205,8 @@ void HELPER(gvec_usadd32)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(uint32_t)) {
>           uint32_t ai = *(uint32_t *)(a + i);
>           uint32_t bi = *(uint32_t *)(b + i);
> -        uint32_t di = ai + bi;
> -        if (di < ai) {
> +        uint32_t di;
> +        if (uadd32_overflow(ai, bi, &di)) {
>               di = UINT32_MAX;
>           }
>           *(uint32_t *)(d + i) = di;
> @@ -1226,8 +1222,8 @@ void HELPER(gvec_usadd64)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(uint64_t)) {
>           uint64_t ai = *(uint64_t *)(a + i);
>           uint64_t bi = *(uint64_t *)(b + i);
> -        uint64_t di = ai + bi;
> -        if (di < ai) {
> +        uint64_t di;
> +        if (uadd64_overflow(ai, bi, &di)) {
>               di = UINT64_MAX;
>           }
>           *(uint64_t *)(d + i) = di;
> @@ -1273,8 +1269,8 @@ void HELPER(gvec_ussub32)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(uint32_t)) {
>           uint32_t ai = *(uint32_t *)(a + i);
>           uint32_t bi = *(uint32_t *)(b + i);
> -        uint32_t di = ai - bi;
> -        if (ai < bi) {
> +        uint32_t di;
> +        if (usub32_overflow(ai, bi, &di)) {
>               di = 0;
>           }
>           *(uint32_t *)(d + i) = di;
> @@ -1290,8 +1286,8 @@ void HELPER(gvec_ussub64)(void *d, void *a, void *b, uint32_t desc)
>       for (i = 0; i < oprsz; i += sizeof(uint64_t)) {
>           uint64_t ai = *(uint64_t *)(a + i);
>           uint64_t bi = *(uint64_t *)(b + i);
> -        uint64_t di = ai - bi;
> -        if (ai < bi) {
> +        uint64_t di;
> +        if (usub64_overflow(ai, bi, &di)) {
>               di = 0;
>           }
>           *(uint64_t *)(d + i) = di;
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb