On Tue, Feb 7, 2017 at 7:32 PM, Jeff Law <l...@redhat.com> wrote: > > This is unchanged from the original posting. Reposting to make review > easier. > > > The tests in g++.dg start with a reduced test from Martin (pr79095-1.C) that > includes a size check. With the size != 0 check this testcase should not > issue any warnings as the path that turns into a __builtin_memset should get > eliminated. > > pr79095-2.C is the same test, but without the size != test. This test > should trigger an "exceeds maximum object size" warning. > > pr79095-3.C is the original test from the BZ, but with a proper size check > on the vector. This test should not produce a warning. > > pr79095-4.C is the original test from the BZ, but without a size check on > the vector. This should produce *one* warning (trunk currently generates > 3). We verify that there's just one __builtin_memset by the time VRP2 is > done and verify that we do get the desired warning. > > pr79095-5.C is another test from Martin which should not produce a warning. > > gcc-torture/execute/arith-1.c is updated to test a few more cases. This was > mis-compiled at some point during patch development and at that time I added > the additional tsts. > > gcc.dg/tree-ssa/pr79095.c is a new test to verify that VRP can propagate > constants generated on the true/false arms of an overflow test and that > constants to _not_ propagate into the wrong arm of the conditional. > > > These were included in the bootstrap & regression testing of the prior > patches. All the tests pass with the prior patches installed. OK for the > trunk?
Ok. Richard. > > * g++.dg/pr79095-1.C: New test > * g++.dg/pr79095-2.C: New test > * g++.dg/pr79095-3.C: New test > * g++.dg/pr79095-4.C: New test > * g++.dg/pr79095-5.C: New test > * gcc.c-torture/execute/arith-1.c: Update with more cases. > * gcc.dg/tree-ssa/pr79095-1.c: New test. > > diff --git a/gcc/testsuite/g++.dg/pr79095-1.C > b/gcc/testsuite/g++.dg/pr79095-1.C > new file mode 100644 > index 0000000..4b8043c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr79095-1.C > @@ -0,0 +1,40 @@ > +/* { dg-do compile } */ > +/* { dg-options "-Wall -O3" } */ > + > +typedef long unsigned int size_t; > + > +inline void > +fill (int *p, size_t n, int) > +{ > + while (n--) > + *p++ = 0; > +} > + > +struct B > +{ > + int* p0, *p1, *p2; > + > + size_t size () const { > + return size_t (p1 - p0); > + } > + > + void resize (size_t n) { > + if (n > size()) > + append (n - size()); > + } > + > + void append (size_t n) > + { > + if (size_t (p2 - p1) >= n) { > + fill (p1, n, 0); > + } > + } > +}; > + > +void foo (B &b) > +{ > + if (b.size () != 0) > + b.resize (b.size () - 1); > +} > + > + > diff --git a/gcc/testsuite/g++.dg/pr79095-2.C > b/gcc/testsuite/g++.dg/pr79095-2.C > new file mode 100644 > index 0000000..9dabc7e > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr79095-2.C > @@ -0,0 +1,46 @@ > +/* { dg-do compile } */ > +/* { dg-options "-Wall -O3" } */ > + > +typedef long unsigned int size_t; > + > +inline void > +fill (int *p, size_t n, int) > +{ > + while (n--) > + *p++ = 0; > +} > + > +struct B > +{ > + int* p0, *p1, *p2; > + > + size_t size () const { > + return size_t (p1 - p0); > + } > + > + void resize (size_t n) { > + if (n > size()) > + append (n - size()); > + } > + > + void append (size_t n) > + { > + if (size_t (p2 - p1) >= n) { > + fill (p1, n, 0); > + } > + } > +}; > + > +void foo (B &b) > +{ > + b.resize (b.size () - 1); > +} > + > +/* If b.size() == 0, then the argument to b.resize is -1U (it overflowed). > + This will result calling "fill" which turns into a memset with a bogus > + length argument. We want to make sure we warn, which multiple > + things. First the ldist pass converted the loop into a memset, > + cprop and simplifications made the length a constant and the static > + analysis pass determines it's a bogus size to pass to memset. */ > +/* { dg-warning "exceeds maximum object size" "" { target *-*-* } 0 } */ > + > diff --git a/gcc/testsuite/g++.dg/pr79095-3.C > b/gcc/testsuite/g++.dg/pr79095-3.C > new file mode 100644 > index 0000000..28c8a37 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr79095-3.C > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-options "-Wall -O3" } */ > + > +#include <vector> > + > +void foo(std::vector<unsigned int> &v); > + > +void vtest() > +{ > + std::vector<unsigned int> v; > + foo (v); > + if (v.size() > 0) > + { > + v.resize (v.size()-1); > + } > +} > + > diff --git a/gcc/testsuite/g++.dg/pr79095-4.C > b/gcc/testsuite/g++.dg/pr79095-4.C > new file mode 100644 > index 0000000..df55025 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr79095-4.C > @@ -0,0 +1,26 @@ > +/* { dg-do compile } */ > +/* { dg-options "-Wall -O3 -fdump-tree-vrp2" } */ > + > +#include <vector> > + > +void foo(std::vector<unsigned int> &v); > + > +void vtest() > +{ > + std::vector<unsigned int> v; > + foo (v); > + { > + v.resize (v.size()-1); > + } > +} > + > +/* As written this testcase should trigger a warning. We overflow to -1U > + if v.size() == 0 in foo(). This results in bogus calls to memset. > + > + The number of clearing loops in the IL can vary depending on the C++ > + mode used for the test. But by the end of VRP2, there should be a > single > + clearing loop left and it should be using memcpy. */ > +/* { dg-final { scan-tree-dump-times "__builtin_memset \\(_\[0-9\]+, 0, > \[0-9\]+\\)" 1 "vrp2" } } */ > + > +/* And that call should trigger a warning. */ > +/* { dg-warning "exceeds maximum object size" "" { target *-*-* } 0 } */ > diff --git a/gcc/testsuite/g++.dg/pr79095-5.C > b/gcc/testsuite/g++.dg/pr79095-5.C > new file mode 100644 > index 0000000..266f4e9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pr79095-5.C > @@ -0,0 +1,34 @@ > +/* { dg-do compile } */ > +/* { dg-options "-Wall -O3" } */ > + > +typedef __SIZE_TYPE__ size_t; > + > +struct S { > + int *p0, *p1, *p2; > + > + size_t size () const { return p1 - p0; } > + > + void f (size_t n) { > + if (n > size ()) // can't happen because > + foo (n - size ()); // n is in [1, MIN(size() - 1, 3)] > + else if (n < size ()) > + bar (p0 + n); > + } > + > + void foo (size_t n) > + { > + size_t left = (size_t)(p2 - p1); > + if (left >= n) > + __builtin_memset (p2, 0, n * sizeof *p2); // { dg-bogus "maximum > object size" } > + > + } > + > + void bar (int*); > +}; > + > +void f (S &s) > +{ > + size_t n = s.size (); > + if (n > 1 && n < 5) > + s.f (n - 1); > +} > diff --git a/gcc/testsuite/gcc.c-torture/execute/arith-1.c > b/gcc/testsuite/gcc.c-torture/execute/arith-1.c > index 58df322..6168d77 100644 > --- a/gcc/testsuite/gcc.c-torture/execute/arith-1.c > +++ b/gcc/testsuite/gcc.c-torture/execute/arith-1.c > @@ -7,9 +7,41 @@ sat_add (unsigned i) > return ret; > } > > +unsigned > +sat_add2 (unsigned i) > +{ > + unsigned ret = i + 1; > + if (ret > i) > + return ret; > + return i; > +} > + > +unsigned > +sat_add3 (unsigned i) > +{ > + unsigned ret = i - 1; > + if (ret > i) > + ret = i; > + return ret; > +} > + > +unsigned > +sat_add4 (unsigned i) > +{ > + unsigned ret = i - 1; > + if (ret < i) > + return ret; > + return i; > +} > main () > { > if (sat_add (~0U) != ~0U) > abort (); > + if (sat_add2 (~0U) != ~0U) > + abort (); > + if (sat_add3 (0U) != 0U) > + abort (); > + if (sat_add4 (0U) != 0U) > + abort (); > exit (0); > } > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr79095.c > b/gcc/testsuite/gcc.dg/tree-ssa/pr79095.c > new file mode 100644 > index 0000000..f635fca > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr79095.c > @@ -0,0 +1,436 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-ipa-icf -fdump-tree-vrp1" } */ > + > +extern void arf (unsigned x, unsigned y); > +extern void baz (unsigned x, unsigned y); > + > +unsigned > +f1 (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (b < a) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > + > +unsigned > +f1r (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (a < b) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f1n (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(b < a)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f1nr (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(a < b)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > + > +unsigned > +f1o (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (a < b) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f1ro (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (b < a) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f1no (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(a < b)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f1nro (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(b < a)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > + > +unsigned > +f2 (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (b <= a) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f2r (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (a <= b) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f2n (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(b <= a)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f2nr (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(a <= b)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > + > +unsigned > +f2o (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (a <= b) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f2ro (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (b <= a) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f2no (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(a <= b)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f2nro (unsigned a, unsigned b) > +{ > + b = a + 1; > + if (!(b <= a)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > + > +unsigned > +f3 (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (b < a) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f3r (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (a < b) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f3n (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(b < a)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f3nr (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(a < b)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > + > +unsigned > +f3o (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (a < b) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f3ro (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (b < a) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f3no (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(a < b)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f3nro (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(b < a)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > + > +unsigned > +f4 (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (b <= a) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f4r (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (a <= b) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f4n (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(b <= a)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f4nr (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(a <= b)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > + > +unsigned > +f4o (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (a <= b) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +unsigned > +f4ro (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (b <= a) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f4no (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(a <= b)) > + { > + baz (a, b); > + return 42; > + } > + arf (a, b); > + return b; > +} > + > +unsigned > +f4nro (unsigned a, unsigned b) > +{ > + b = a - 1; > + if (!(b <= a)) > + { > + arf (a, b); > + return 42; > + } > + baz (a, b); > + return b; > +} > + > +/* All calls to baz should still reference a & b as arguments. */ > +/* { dg-final { scan-tree-dump-times "baz \\(a_\[0-9\]+\\(D\\), > b_\[0-9\]+\\)" 32 "vrp1"} } */ > + > + > +/* All calls to arf should have constant arguments. */ > +/* { dg-final { scan-tree-dump-times "arf \\(\[0-9\]+, \[0-9\]+\\)" 32 > "vrp1"} } */ >