Re: [PATCH 1/2] target/s390x: Fix determination of overflow condition code after addition

[Richard Henderson]

On 3/30/22 02:52, David Hildenbrand wrote:
> >   static uint32_t cc_calc_add_64(int64_t a1, int64_t a2, int64_t ar)
> >   {
> > -    if ((a1 > 0 && a2 > 0 && ar < 0) || (a1 < 0 && a2 < 0 && ar > 0)) {
> > +    if ((a1 > 0 && a2 > 0 && ar < 0) || (a1 < 0 && a2 < 0 && ar >= 0)) {
>
>
> Intuitively, I'd have checked for any overflow/underflow by comparing
> with one of the input variables:
>
> a) Both numbers are positive
>
> Adding to positive numbers has to result in something that's bigger than
> the input parameters.
>
> "a1 > 0 && a2 > 0 && ar < a1"
>
> b) Both numbers are negative
>
> Adding to negative numbers has to result in something that's smaller
> than the input parameters.
>
> "a1 < 0 && a2 < 0 && ar > a1"

If we're not going to just fix the >= typo,
I'd suggest using the much more compact bitwise method:

    ((ar ^ a1) & ~(a1 ^ a2)) < 0

See sadd64_overflow in qemu/host-utils.h.


r~