Re: [PATCH] Make CCP effectively remove UNDEFINED defs

[Richard Biener <rguenther at suse dot de>]

On Wed, 4 Nov 2015, Marc Glisse wrote:

> On Wed, 4 Nov 2015, Richard Biener wrote:
> 
> > On Wed, 4 Nov 2015, Richard Biener wrote:
> > 
> > > On Wed, 4 Nov 2015, Marc Glisse wrote:
> > > 
> > > > On Wed, 4 Nov 2015, Richard Biener wrote:
> > > > 
> > > > > The following patch makes sure that CCP when it computes a lattice
> > > > > value to UNDEFINED ends up replacing uses with default defs (and thus
> > > > > removes such UNDEFINED producing defs).  This optimizes the testcase
> > > > > below to
> > > > > 
> > > > >  <bb 2>:
> > > > >  return _6(D);
> > > > > 
> > > > > in the first CCP.  Note this patch isn't mainly for the optimization
> > > > > it does but for making it easier to discover cases where CCP gets
> > > > > UNDEFINED wrong (similar to VRP2 re-using the range-info that is
> > > > > still live on SSA names - that's to catch bogus range-info).
> > > > > 
> > > > > If the experiment works out (read: bootstrap / test completes
> > > > > successfully) I'm going to enhance VRP and will also look at
> > > > > how value-numbering could do a similar job.
> > > > 
> > > > Cool, the VN part might help with
> > > > https://gcc.gnu.org/ml/gcc-patches/2015-05/msg01556.html
> > > > :-)
> > > 
> > > Actually it "breaks" gcc.dg/tree-ssa/pr44133.c as it transforms
> > > 
> > >   <bb 2>:
> > >   _2 = l_1(D) + -22;
> > >   _4 = _2 + s$i_5(D);
> > >   return _4;
> > > 
> > > into
> > > 
> > >   <bb 2>:
> > >   return _6(D);
> > > 
> > > turning the warning of an uninitialized use of s$i_5(D) (aka s.i)
> > > into a warning of an uninitialized use of '({anonymous})'.
> > > 
> > > So I wonder if a better approach is to handle UNDEFINED differently
> > > during propagation (now that we can propagate copies) and make
> > > the value of _4 not UNDEFINED but a copy of (one of) the operand
> > > that makes it undefined.  Thus we'd turn the above into
> > > 
> > >   <bb 2>:
> > >   return s$i_5(D);
> > > 
> > > there is the chance that the warnings resulting from this will
> > > be confusing (warning for an uninitialized use of sth far away
> > > from its actual use).  Of course the above propagation is also
> > > cheaper to implement (apart from the choice you have when
> > > seeing x_1(D) + y_2(D) - which one do you pick and retain
> > > the warning for?)
> > 
> > Ok, it's actually _not_ easy.  The operand might not have the
> > correct type after all and we'll introduce a not fully propagated
> > lattice that way (the 'copy' value would have a lattice value
> > of UNDEFINED).
> 
> If you create a new variable (that _6 is a default value for), you could also
> give it the same name (gcc may append .1) and location as s.i, even if the
> type differs. It wouldn't be perfect (might be too much of a hack for gcc),
> but should be more readable than '({anonymous})'. Hmm, I now realize that it
> doesn't really help, because you would have to maintain somehow the
> information of which variable's name to copy, and that's what you are saying
> is hard :-( Sorry for the useless comment.

No problem.  I'm holding off on this for now (similar to how I did on
optimistically propagating copies ...).  The uninit var stuff is bad
and I have no time to sort this out before stage1 ends.

Anyway, the following fixes the testcases I found with the patch ;)

Will commit after testing.

Richard.

2015-11-04  Richard Biener  <rguenther@suse.de>

	* gcc.dg/tree-ssa/loadpre2.c: Avoid undefined behavior due to
	uninitialized variables.
	* gcc.dg/tree-ssa/loadpre21.c: Likewise.
	* gcc.dg/tree-ssa/loadpre22.c: Likewise.
	* gcc.dg/tree-ssa/loadpre23.c: Likewise.
	* gcc.dg/tree-ssa/loadpre24.c: Likewise.
	* gcc.dg/tree-ssa/loadpre25.c: Likewise.
	* gcc.dg/tree-ssa/loadpre4.c: Likewise.
	* gcc.dg/ipa/inlinehint-2.c: Likewise.
	* gcc.dg/ipa/pure-const-2.c: Likewise.
	* gcc.dg/tree-ssa/loop-1.c: Likewise.
	* gcc.dg/tree-ssa/loop-23.c: Likewise.
	* gcc.dg/tree-ssa/pr22051-2.c: Likewise.
	* gcc.dg/tree-ssa/ssa-pre-3.c: Likewise.
	* gcc.dg/tree-ssa/ssa-sccvn-3.c: Likewise.
	* gcc.dg/vect/pr30858.c: Likewise.
	* gcc.dg/vect/pr33866.c: Likewise.
	* gcc.dg/vect/pr37027.c: Likewise.
	* c-c++-common/ubsan/null-10.c: Likewise.
	* gcc.target/i386/incoming-8.c: Likewise.

Index: gcc/testsuite/c-c++-common/ubsan/null-10.c
===================================================================
--- gcc/testsuite/c-c++-common/ubsan/null-10.c	(revision 229752)
+++ gcc/testsuite/c-c++-common/ubsan/null-10.c	(working copy)
@@ -2,10 +2,12 @@
 /* { dg-options "-fsanitize=null -w" } */
 /* { dg-shouldfail "ubsan" } */
 
+short x;
+
 int
 main (void)
 {
-  short *p = 0, *u;
+  short *p = 0, *u = &x;
   *(u + *p) = 23;
   return  0;
 }
Index: gcc/testsuite/gcc.dg/ipa/inlinehint-2.c
===================================================================
--- gcc/testsuite/gcc.dg/ipa/inlinehint-2.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/ipa/inlinehint-2.c	(working copy)
@@ -5,7 +5,7 @@ int
 t(int s, void **p)
 {
   int i;
-  for (i;i<10000;i+=s)
+  for (i=0;i<10000;i+=s)
     p[i]=0;
 }
 int
Index: gcc/testsuite/gcc.dg/ipa/pure-const-2.c
===================================================================
--- gcc/testsuite/gcc.dg/ipa/pure-const-2.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/ipa/pure-const-2.c	(working copy)
@@ -5,7 +5,7 @@ int i_am_pure(char *c, int n)
 {
   char *d=__builtin_alloca (n);
   int i;
-  int sum;
+  int sum = 0;
   for (i=0;i<n;i++)
     d[i] = c[i];
   for (i=0;i<n;i++)
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre2.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre2.c	(working copy)
@@ -2,16 +2,15 @@
 /* { dg-options "-O2 -fdump-tree-pre-stats" } */
 int main(int *a, int argc)
 {
-  int b;
   int i;
-  int d, e;
+  int e;
 
   /* Should be able to hoist this out of the loop.  */
   for (i = 0; i < argc; i++)
     {
       e = *a;
     }
-  return d + e;
+  return e;
 }
 
 /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"} } */
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre21.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre21.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre21.c	(working copy)
@@ -3,16 +3,15 @@
 typedef int type[2];
 int main(type *a, int argc)
 {
-  int b;
   int i;
-  int d, e;
+  int e;
 
   /* Should be able to hoist this out of the loop.  */
   for (i = 0; i < argc; i++)
     {
       e = (*a)[0];
     }
-  return d + e;
+  return e;
 }
 
 /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"} } */
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre22.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre22.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre22.c	(working copy)
@@ -3,16 +3,15 @@
 typedef int type[2];
 int main(type *a, int argc)
 {
-  int b;
   int i;
-  int d, e;
+  int e;
 
   /* Should be able to hoist this out of the loop.  */
   for (i = 0; i < argc; i++)
     {
       e = (*a)[argc];
     }
-  return d + e;
+  return e;
 }
 
 /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"} } */
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre23.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre23.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre23.c	(working copy)
@@ -8,17 +8,16 @@ struct {
 
 int foo(int argc)
 {
-  int b;
   int c;
   int i;
-  int d, e;
+  int e;
 
   for (i = 0; i < argc; i++)
     {
       e = x.a;
       x.a = 9;
     }
-  return d + e;
+  return e;
 }
 
 /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"  } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre24.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre24.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre24.c	(working copy)
@@ -5,17 +5,15 @@ int a;
 
 int foo(int argc)
 {
-  int b;
-  int c;
   int i;
-  int d, e;
+  int e;
 
   for (i = 0; i < argc; i++)
     {
       e = a;
       a = 9;
     }
-  return d + e;
+  return e;
 }
 
 /* We will move the load of a out of the loop.  */
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre25.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre25.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre25.c	(working copy)
@@ -3,17 +3,15 @@
 struct X { int i; };
 int foo(struct X *a, int argc)
 {
-  int b;
-  int c;
   int i;
-  int d, e;
+  int e;
 
   for (i = 0; i < argc; i++)
     {
       e = a->i;
       a->i = 9;
     }
-  return d + e;
+  return e;
 }
 
 /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"  } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre4.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loadpre4.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loadpre4.c	(working copy)
@@ -2,17 +2,15 @@
 /* { dg-options "-O2 -fdump-tree-pre-stats" } */
 int main(int *a, int argc)
 {
-  int b;
-  int c;
   int i;
-  int d, e;
+  int e;
 
   for (i = 0; i < argc; i++)
     {
       e = *a;
       *a = 9;
     }
-  return d + e;
+  return e;
 }
 
 /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"  } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/loop-1.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loop-1.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loop-1.c	(working copy)
@@ -24,7 +24,7 @@ int foo (int x);
 int xxx(void)
 {
   int x = 45;
-  int sum;
+  int sum = 0;
 
   while (x >>= 1)
     sum += foo (x) * 2;
Index: gcc/testsuite/gcc.dg/tree-ssa/loop-23.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/loop-23.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/loop-23.c	(working copy)
@@ -7,7 +7,7 @@ int bla(int);
 int foo(void)
 {
   int i;
-  int sum;
+  int sum = 0;
 
   /* This loop used to appear to be too large for unrolling.  */
   for (i = 0; i < 4; i++)
Index: gcc/testsuite/gcc.dg/tree-ssa/pr22051-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr22051-2.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr22051-2.c	(working copy)
@@ -1,14 +1,10 @@
 /* { dg-do compile }  */
 /* { dg-options "-O2 -fdump-tree-optimized -w" }  */
 
-
-
-
 void *arf ();
 int
-foo()
+foo(void (*q)(void))
 {
-  void (*q)(void);
   int r = q;
 
   if (r != 0)
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-3.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-3.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-3.c	(working copy)
@@ -2,7 +2,7 @@
 /* { dg-options "-O2 -fdump-tree-pre-stats" } */
 unsigned foo1 (unsigned a, unsigned b)
 {
-  unsigned i, j, k;
+  unsigned i, j = 0, k = 0;
   for (i = 0; i != a; i++)
     {
       j += 4*b;
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-3.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-3.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-sccvn-3.c	(working copy)
@@ -1,8 +1,8 @@
 /* { dg-do compile } */ 
-/* { dg-options "-O2 -fdump-tree-fre1-stats" } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+int *p;
 int main(int argc, char **argv)
 {
-  int *p;
   int result;
   *p = 2;
   if (argc)
@@ -11,4 +11,4 @@ int main(int argc, char **argv)
   return result;
 }
 /* We should eliminate result = *p by saying it has the value 2.  */
-/* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "fre1"} } */
+/* { dg-final { scan-tree-dump "return 2;" "fre1"} } */
Index: gcc/testsuite/gcc.dg/vect/pr30858.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/pr30858.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/vect/pr30858.c	(working copy)
@@ -4,7 +4,7 @@
 int
 foo (int ko)
 {
- int j,i;
+ int j,i = 0;
   for (j = 0; j < ko; j++)
    i += (i > 10) ? -5 : 7;
  return i;
Index: gcc/testsuite/gcc.dg/vect/pr33866.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/pr33866.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/vect/pr33866.c	(working copy)
@@ -18,7 +18,7 @@ void test_select_fill_hyper_simple (long
   fill_iter_info iter_info;
   int i, j;
   iter_info.coords = (long *) points;
-  for (i = 0, num_points = 0; j < (int) start[1]; j++, num_points++)
+  for (j = i = 0, num_points = 0; j < (int) start[1]; j++, num_points++)
   {
     points[num_points][0] = i + start[0];
     points[num_points][1] = j + start[1];
Index: gcc/testsuite/gcc.dg/vect/pr37027.c
===================================================================
--- gcc/testsuite/gcc.dg/vect/pr37027.c	(revision 229752)
+++ gcc/testsuite/gcc.dg/vect/pr37027.c	(working copy)
@@ -18,8 +18,8 @@ void
 foo (void)
 {
   int i;
-  int sum1;
-  int sum2;
+  int sum1 = 0;
+  int sum2 = 0;
 
   for (i = 0; i < 16; i++)
   {
Index: gcc/testsuite/gcc.target/i386/incoming-8.c
===================================================================
--- gcc/testsuite/gcc.target/i386/incoming-8.c	(revision 229752)
+++ gcc/testsuite/gcc.target/i386/incoming-8.c	(working copy)
@@ -6,7 +6,7 @@ float
 foo (float f)
 {
   float array[128];
-  float x;
+  float x = 0.;
   int i;
   for (i = 0; i < sizeof(array) / sizeof(*array); i++)
     array[i] = f;