Re: calculate overflow type in wide int arithmetic

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Jul 5, 2018 at 9:35 AM Aldy Hernandez <aldyh@redhat.com> wrote:
>
> The reason for this patch are the changes showcased in tree-vrp.c.
> Basically I'd like to discourage rolling our own overflow and underflow
> calculation when doing wide int arithmetic.  We should have a
> centralized place for this, that is-- in the wide int code itself ;-).
>
> The only cases I care about are plus/minus, which I have implemented,
> but we also get division for free, since AFAICT, division can only
> positive overflow:
>
>         -MIN / -1 => +OVERFLOW
>
> Multiplication OTOH, can underflow, but I've not implemented it because
> we have no uses for it.  I have added a note in the code explaining this.
>
> Originally I tried to only change plus/minus, but that made code that
> dealt with plus/minus in addition to div or mult a lot uglier.  You'd
> have to special case "int overflow_for_add_stuff" and "bool
> overflow_for_everything_else".  Changing everything to int, makes things
> consistent.
>
> Note: I have left poly-int as is, with its concept of yes/no for
> overflow.  I can adapt this as well if desired.
>
> Tested on x86-64 Linux.
>
> OK for trunk?

looks all straight-forward but the following:

   else if (op1)
     {
       if (minus_p)
-       {
-         wi = -wi::to_wide (op1);
-
-         /* Check for overflow.  */
-         if (sgn == SIGNED
-             && wi::neg_p (wi::to_wide (op1))
-             && wi::neg_p (wi))
-           ovf = 1;
-         else if (sgn == UNSIGNED && wi::to_wide (op1) != 0)
-           ovf = -1;
-       }
+       wi = wi::neg (wi::to_wide (op1));
       else
        wi = wi::to_wide (op1);

you fail to handle - -INT_MIN.

Given the fact that for multiplication (or others, didn't look too  close)
you didn't implement the direction indicator I wonder if it would be more
appropriate to do

enum ovfl { OVFL_NONE = 0, OVFL_UNDERFLOW = -1, OVFL_OVERFLOW = 1,
OVFL_UNKNOWN = 2 };

and tell us the "truth" here?

Hopefully if (overflow) will still work with that.

Otherwise can you please add a toplevel comment to wide-int.h as to what the
overflow result semantically is for a) SIGNED and b) UNSIGNED operations?

Thanks,
Richard.