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

[Richard Henderson]

On 3/30/22 02:57, David Hildenbrand wrote:
> > diff --git a/target/s390x/tcg/cc_helper.c b/target/s390x/tcg/cc_helper.c
> > index e11cdb745d..b2e8d3d9f5 100644
> > --- a/target/s390x/tcg/cc_helper.c
> > +++ b/target/s390x/tcg/cc_helper.c
> > @@ -151,7 +151,7 @@ static uint32_t cc_calc_add_64(int64_t a1, int64_t a2, 
> > int64_t ar)
> >   
> >   static uint32_t cc_calc_sub_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)) {
> >           return 3; /* overflow */
> >       } else {
> >           if (ar < 0) {
> > @@ -211,7 +211,7 @@ static uint32_t cc_calc_add_32(int32_t a1, int32_t a2, 
> > int32_t ar)
> >   
> >   static uint32_t cc_calc_sub_32(int32_t a1, int32_t a2, int32_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)) {
> >           return 3; /* overflow */
> >       } else {
> >           if (ar < 0) {
>
> Again, intuitively I'd check for
>
> a) Subtracting a negative number from a positive one -> Adding two
> positive numbers should result in the result being bigger than the first
> parameter.
>
> a1 > 0 && a2 < 0 && ar < a1
>
> a) Subtracting a positive number from a negative one -> Adding two
> negative numbers should result in something that's smaller than the
> first parameter
>
> a1 < 0 && a2 > 0 && ar > a1
>
>
> But maybe I am missing something :)

Again, see ssub64_overflow in qemu/host-utils.h.


r~