[PATCH] Fix VRP update_value_range and caller (PR tree-optimization/65875)

[PATCH] Fix VRP update_value_range and caller (PR tree-optimization/65875)

[Jakub Jelinek]

Hi!

In vrp_visit_assignment_or_call we try to return SSA_PROP_VARYING
if update_value_range returned true and the new range is VR_VARYING,
but vrp_visit_phi_node fails to do that.
Another thing is that if update_value_range decides to
set_value_range_to_varying (old_vr);
it doesn't update new_vr, so the caller doesn't know the new vr
is varying (and calling get_value_range again sounds unnecessary).

The following patch fixes it, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk and 5.2?

2015-04-24  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/65875
	* tree-vrp.c (update_value_range): If in is_new case setting
	old_vr to VR_VARYING, also set new_vr to it.
	(vrp_visit_phi_node): Return SSA_PROP_VARYING instead of
	SSA_PROP_INTERESTING if update_value_range returned true,
	but new range is VR_VARYING.

	* gcc.c-torture/compile/pr65875.c: New test.

--- gcc/tree-vrp.c.jj	2015-04-20 14:35:39.000000000 +0200
+++ gcc/tree-vrp.c	2015-04-24 18:10:41.321367440 +0200
@@ -892,7 +892,12 @@ update_value_range (const_tree var, valu
 	 UNDEFINED or from VARYING.  */
       if (new_vr->type == VR_UNDEFINED
 	  || old_vr->type == VR_VARYING)
-	set_value_range_to_varying (old_vr);
+	{
+	  BITMAP_FREE (new_vr->equiv);
+	  set_value_range_to_varying (old_vr);
+	  set_value_range_to_varying (new_vr);
+	  return true;
+	}
       else
 	set_value_range (old_vr, new_vr->type, new_vr->min, new_vr->max,
 			 new_vr->equiv);
@@ -8941,6 +8946,9 @@ update_range:
 	  fprintf (dump_file, "\n");
 	}
 
+      if (vr_result.type == VR_VARYING)
+	return SSA_PROP_VARYING;
+
       return SSA_PROP_INTERESTING;
     }
 
--- gcc/testsuite/gcc.c-torture/compile/pr65875.c.jj	2015-04-24 18:20:47.650595581 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr65875.c	2015-04-24 18:20:30.000000000 +0200
@@ -0,0 +1,24 @@
+/* PR tree-optimization/65875 */
+
+int a, b, c, d, e, f, g;
+
+void
+foo (void)
+{
+  long h = 0, i;
+  if (g < 0)
+    i = -g;
+  for (; b;)
+    for (; c;)
+      if (e)
+	h = 1;
+  for (; f;)
+    if (a)
+      break;
+  if (h > i)
+    while (h > i)
+      {
+	d = 0;
+	h--;
+      }
+}

	Jakub

Re: [PATCH] Fix VRP update_value_range and caller (PR tree-optimization/65875)

[Richard Biener]

On Fri, 24 Apr 2015, Jakub Jelinek wrote:

> Hi!
> 
> In vrp_visit_assignment_or_call we try to return SSA_PROP_VARYING
> if update_value_range returned true and the new range is VR_VARYING,
> but vrp_visit_phi_node fails to do that.
> Another thing is that if update_value_range decides to
> set_value_range_to_varying (old_vr);
> it doesn't update new_vr, so the caller doesn't know the new vr
> is varying (and calling get_value_range again sounds unnecessary).
> 
> The following patch fixes it, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk and 5.2?
> 
> 2015-04-24  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/65875
> 	* tree-vrp.c (update_value_range): If in is_new case setting
> 	old_vr to VR_VARYING, also set new_vr to it.
> 	(vrp_visit_phi_node): Return SSA_PROP_VARYING instead of
> 	SSA_PROP_INTERESTING if update_value_range returned true,
> 	but new range is VR_VARYING.
> 
> 	* gcc.c-torture/compile/pr65875.c: New test.
> 
> --- gcc/tree-vrp.c.jj	2015-04-20 14:35:39.000000000 +0200
> +++ gcc/tree-vrp.c	2015-04-24 18:10:41.321367440 +0200
> @@ -892,7 +892,12 @@ update_value_range (const_tree var, valu
>  	 UNDEFINED or from VARYING.  */
>        if (new_vr->type == VR_UNDEFINED
>  	  || old_vr->type == VR_VARYING)
> -	set_value_range_to_varying (old_vr);
> +	{
> +	  BITMAP_FREE (new_vr->equiv);
> +	  set_value_range_to_varying (old_vr);
> +	  set_value_range_to_varying (new_vr);
> +	  return true;

Actually we didn't change anything here (old_vr->type is VARYING already,
so we shouldn't even have visited the stmt defining the SSA name again...
eventually your fix below fixes that.

So I'd prefer to simply drop the hunk... (you might want to do some
digging which rev. introduced it and if it came with a testcase...).

Otherwise ok.

Thanks,
Richard.

> +	}
>        else
>  	set_value_range (old_vr, new_vr->type, new_vr->min, new_vr->max,
>  			 new_vr->equiv);
> @@ -8941,6 +8946,9 @@ update_range:
>  	  fprintf (dump_file, "\n");
>  	}
>  
> +      if (vr_result.type == VR_VARYING)
> +	return SSA_PROP_VARYING;
> +
>        return SSA_PROP_INTERESTING;
>      }
>  
> --- gcc/testsuite/gcc.c-torture/compile/pr65875.c.jj	2015-04-24 18:20:47.650595581 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr65875.c	2015-04-24 18:20:30.000000000 +0200
> @@ -0,0 +1,24 @@
> +/* PR tree-optimization/65875 */
> +
> +int a, b, c, d, e, f, g;
> +
> +void
> +foo (void)
> +{
> +  long h = 0, i;
> +  if (g < 0)
> +    i = -g;
> +  for (; b;)
> +    for (; c;)
> +      if (e)
> +	h = 1;
> +  for (; f;)
> +    if (a)
> +      break;
> +  if (h > i)
> +    while (h > i)
> +      {
> +	d = 0;
> +	h--;
> +      }
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

Re: [PATCH] Fix VRP update_value_range and caller (PR tree-optimization/65875)

[Jakub Jelinek]

On Mon, Apr 27, 2015 at 10:05:21AM +0200, Richard Biener wrote:
> > --- gcc/tree-vrp.c.jj	2015-04-20 14:35:39.000000000 +0200
> > +++ gcc/tree-vrp.c	2015-04-24 18:10:41.321367440 +0200
> > @@ -892,7 +892,12 @@ update_value_range (const_tree var, valu
> >  	 UNDEFINED or from VARYING.  */
> >        if (new_vr->type == VR_UNDEFINED
> >  	  || old_vr->type == VR_VARYING)
> > -	set_value_range_to_varying (old_vr);
> > +	{
> > +	  BITMAP_FREE (new_vr->equiv);
> > +	  set_value_range_to_varying (old_vr);
> > +	  set_value_range_to_varying (new_vr);
> > +	  return true;
> 
> Actually we didn't change anything here (old_vr->type is VARYING already,
> so we shouldn't even have visited the stmt defining the SSA name again...
> eventually your fix below fixes that.

On the testcase, old_vr wasn't actually VARYING, but new_vr was UNDEFINED
(a result of intersecting disjoint ranges).  While for old_vr->type ==
VR_VARYING I agree we shouldn't have been called on this.

And turning a VR_RANGE into VR_UNDEFINED is going in the wrong direction
in the lattice.

	Jakub

Re: [PATCH] Fix VRP update_value_range and caller (PR tree-optimization/65875)

[Richard Biener]

On Mon, 27 Apr 2015, Jakub Jelinek wrote:

> On Mon, Apr 27, 2015 at 10:05:21AM +0200, Richard Biener wrote:
> > > --- gcc/tree-vrp.c.jj	2015-04-20 14:35:39.000000000 +0200
> > > +++ gcc/tree-vrp.c	2015-04-24 18:10:41.321367440 +0200
> > > @@ -892,7 +892,12 @@ update_value_range (const_tree var, valu
> > >  	 UNDEFINED or from VARYING.  */
> > >        if (new_vr->type == VR_UNDEFINED
> > >  	  || old_vr->type == VR_VARYING)
> > > -	set_value_range_to_varying (old_vr);
> > > +	{
> > > +	  BITMAP_FREE (new_vr->equiv);
> > > +	  set_value_range_to_varying (old_vr);
> > > +	  set_value_range_to_varying (new_vr);
> > > +	  return true;
> > 
> > Actually we didn't change anything here (old_vr->type is VARYING already,
> > so we shouldn't even have visited the stmt defining the SSA name again...
> > eventually your fix below fixes that.
> 
> On the testcase, old_vr wasn't actually VARYING, but new_vr was UNDEFINED
> (a result of intersecting disjoint ranges).  While for old_vr->type ==
> VR_VARYING I agree we shouldn't have been called on this.
> 
> And turning a VR_RANGE into VR_UNDEFINED is going in the wrong direction
> in the lattice.

Ah, I misread the || for a &&.  The patch is ok with just dropping
the == VR_VARYING check.

Thanks,
Richard.