Re: PATCH RFC: Change some tree-ssa-operands fields to unsigned

[Ian Lance Taylor <iant at google dot com>]

Andrew Pinski <pinskia@physics.uc.edu> writes:

> > tree-ssa-operand.h has macros with code like this:
> >     gcc_assert ((X) >= 0 && (X) < VUSE_VECT_NUM_ELEM (V)),
> > These are often used in loops.  In a loop VRP can often eliminate the
> > (X) >= 0 test.  However, when VRP does this, it is relying on
> > undefined signed overflow: it is assuming that the loop will not wrap.
> > This generates extra warnings with a warning I am working on for cases
> > where gcc optimizers rely on undefined signed overflow.
> 
> This is exactly where I was talkng about this warning and bounds checking code.
> I think the warning is over board in this case, yes we depend on overflow being
> undefined here but this case is exactly the case whoch VRP is designed to optimize.
> Also didn't we decide that loop based signed integer overflow is ok if it was
> undefined?  So I don't see why we should warn here.

I feel that we do not need to warn about using undefined signed
overflow when determining things like whether a loop terminates and
how many iterations a loop executes.  That is not what is going on
here.  VRP simulates execution of the program.  When it sees a
variable being incremented or decremented in a loop, it sets the range
of that variable to positive or negative infinity.  When VRP later
uses that range in determining whether it can apply an optimization,
it is relying on undefined signed overflow.  This is a much more
general notion than using undefined signed overflow to limit loop
bounds.

> Oh by the way I looked through the source and did not see that style of fo loop.
> 
> so the true question is do we warning for the following function:
> 
> int f(int n)
> {
>   int i;
>   for(i =0 ;i < n;i++)
>   {
>     if (!(i>=0 && i < n) )
>       __builtin_abort ();
>     if (!(i>=0 && i < n) )
>       __builtin_abort ();
>   }
> }
> 
> Because this exactly represents the code in tree-ssa-operands that I could find.
> This shows up a lot in bounds checking code so warning on this case seems wrong
> even if we depend on signed integer overflow being undefined here.

My patch issues no warning for code like the above.

My patch issues a warning for code like this:

#include <stdlib.h>
int g;
int
f (int n)
{
  int i;

  i = 1;
  while (1)
    {
      ++i;
      if (i <= 0)
	abort ();
      if (i >= g)
	break;
    }
  return g;
}

See op_iter_next_use and the comparisons against VUSE_NUM and
VDEF_NUM.


In any case, I am not too interested at this time at debating the
details of the warning and how it works.  I am interested in hearing
comments on the specific patch that I proposed.  Thanks.

Ian