[PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Marek Polacek]

As Richi said in <https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
using recorded SSA name range infos in VRP is likely to expose errors in the
ranges.  This PR is such a case.  As discussed in the PR, after tail merging
via PRE the range infos cannot be relied upon anymore, so we need to clear
them.

Since tree-ssa-ifcombine.c already had code to clean up the flow data in a BB,
I've factored it out to a common function.

Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?

2015-09-24  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/67690
	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
	reset_flow_sensitive_info_in_bb.
	* tree-ssa-tail-merge.c (replace_block_by): Likewise.
	* tree-ssanames.c: Include "gimple-iterator.h".
	(reset_flow_sensitive_info_in_bb): New function.
	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.

	* gcc.dg/torture/pr67690.c: New test.

diff --git gcc/testsuite/gcc.dg/torture/pr67690.c gcc/testsuite/gcc.dg/torture/pr67690.c
index e69de29..491de51 100644
--- gcc/testsuite/gcc.dg/torture/pr67690.c
+++ gcc/testsuite/gcc.dg/torture/pr67690.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+const int c1 = 1;
+const int c2 = 2;
+
+int
+check (int i)
+{
+  int j;
+  if (i >= 0)
+    j = c2 - i;
+  else
+    j = c2 - i;
+  return c2 - c1 + 1 > j;
+}
+
+int invoke (int *pi) __attribute__ ((noinline,noclone));
+int
+invoke (int *pi)
+{
+  return check (*pi);
+}
+
+int
+main ()
+{
+  int i = c1;
+  int ret = invoke (&i);
+  if (!ret)
+    __builtin_abort ();
+  return 0;
+}
diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c
index 9f04174..66be430 100644
--- gcc/tree-ssa-ifcombine.c
+++ gcc/tree-ssa-ifcombine.c
@@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun)
 	  {
 	    /* Clear range info from all stmts in BB which is now executed
 	       conditional on a always true/false condition.  */
-	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
-		 !gsi_end_p (gsi); gsi_next (&gsi))
-	      {
-		gimple *stmt = gsi_stmt (gsi);
-		ssa_op_iter i;
-		tree op;
-		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
-		  reset_flow_sensitive_info (op);
-	      }
-
+	    reset_flow_sensitive_info_in_bb (bb);
 	    cfg_changed |= true;
 	  }
     }
diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c
index 0ce59e8..487961e 100644
--- gcc/tree-ssa-tail-merge.c
+++ gcc/tree-ssa-tail-merge.c
@@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block bb2)
       e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
     }
 
+  /* Clear range info from all stmts in BB2 -- this transformation
+     could make them out of date.  */
+  reset_flow_sensitive_info_in_bb (bb2);
+
   /* Do updates that use bb1, before deleting bb1.  */
   release_last_vdef (bb1);
   same_succ_flush_bb (bb1);
diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
index 4199290..5393865 100644
--- gcc/tree-ssanames.c
+++ gcc/tree-ssanames.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "backend.h"
 #include "tree.h"
 #include "gimple.h"
+#include "gimple-iterator.h"
 #include "hard-reg-set.h"
 #include "ssa.h"
 #include "alias.h"
@@ -544,6 +545,21 @@ reset_flow_sensitive_info (tree name)
     SSA_NAME_RANGE_INFO (name) = NULL;
 }
 
+/* Clear all flow sensitive data from all statements in BB.  */
+
+void
+reset_flow_sensitive_info_in_bb (basic_block bb)
+{
+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+       gsi_next (&gsi))
+    {
+      gimple *stmt = gsi_stmt (gsi);
+      ssa_op_iter i;
+      tree op;
+      FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
+	reset_flow_sensitive_info (op);
+    }
+}
 
 /* Release all the SSA_NAMEs created by STMT.  */
 
diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h
index 22ff609..5688ca5 100644
--- gcc/tree-ssanames.h
+++ gcc/tree-ssanames.h
@@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
 					   struct range_info_def *);
 extern void reset_flow_sensitive_info (tree);
+extern void reset_flow_sensitive_info_in_bb (basic_block);
 extern void release_defs (gimple *);
 extern void replace_ssa_name_symbol (tree, tree);
 

	Marek

Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Richard Biener]

On Thu, 24 Sep 2015, Marek Polacek wrote:

> As Richi said in <https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
> using recorded SSA name range infos in VRP is likely to expose errors in the
> ranges.  This PR is such a case.  As discussed in the PR, after tail merging
> via PRE the range infos cannot be relied upon anymore, so we need to clear
> them.
> 
> Since tree-ssa-ifcombine.c already had code to clean up the flow data in a BB,
> I've factored it out to a common function.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?

I believe for tail-merge you also need to clear range info on
PHI defs in the BB.  For ifcombine this wasn't necessary (no PHI nodes
in the relevant CFG), but it's ok to extend the new 
reset_flow_sensitive_info_in_bb function to also reset PHI defs.

Ok with that change.

Thanks,
Richard.

> 2015-09-24  Marek Polacek  <polacek@redhat.com>
> 
> 	PR tree-optimization/67690
> 	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
> 	reset_flow_sensitive_info_in_bb.
> 	* tree-ssa-tail-merge.c (replace_block_by): Likewise.
> 	* tree-ssanames.c: Include "gimple-iterator.h".
> 	(reset_flow_sensitive_info_in_bb): New function.
> 	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.
> 
> 	* gcc.dg/torture/pr67690.c: New test.
> 
> diff --git gcc/testsuite/gcc.dg/torture/pr67690.c gcc/testsuite/gcc.dg/torture/pr67690.c
> index e69de29..491de51 100644
> --- gcc/testsuite/gcc.dg/torture/pr67690.c
> +++ gcc/testsuite/gcc.dg/torture/pr67690.c
> @@ -0,0 +1,32 @@
> +/* { dg-do run } */
> +
> +const int c1 = 1;
> +const int c2 = 2;
> +
> +int
> +check (int i)
> +{
> +  int j;
> +  if (i >= 0)
> +    j = c2 - i;
> +  else
> +    j = c2 - i;
> +  return c2 - c1 + 1 > j;
> +}
> +
> +int invoke (int *pi) __attribute__ ((noinline,noclone));
> +int
> +invoke (int *pi)
> +{
> +  return check (*pi);
> +}
> +
> +int
> +main ()
> +{
> +  int i = c1;
> +  int ret = invoke (&i);
> +  if (!ret)
> +    __builtin_abort ();
> +  return 0;
> +}
> diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c
> index 9f04174..66be430 100644
> --- gcc/tree-ssa-ifcombine.c
> +++ gcc/tree-ssa-ifcombine.c
> @@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun)
>  	  {
>  	    /* Clear range info from all stmts in BB which is now executed
>  	       conditional on a always true/false condition.  */
> -	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
> -		 !gsi_end_p (gsi); gsi_next (&gsi))
> -	      {
> -		gimple *stmt = gsi_stmt (gsi);
> -		ssa_op_iter i;
> -		tree op;
> -		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
> -		  reset_flow_sensitive_info (op);
> -	      }
> -
> +	    reset_flow_sensitive_info_in_bb (bb);
>  	    cfg_changed |= true;
>  	  }
>      }
> diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c
> index 0ce59e8..487961e 100644
> --- gcc/tree-ssa-tail-merge.c
> +++ gcc/tree-ssa-tail-merge.c
> @@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block bb2)
>        e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
>      }
>  
> +  /* Clear range info from all stmts in BB2 -- this transformation
> +     could make them out of date.  */
> +  reset_flow_sensitive_info_in_bb (bb2);
> +
>    /* Do updates that use bb1, before deleting bb1.  */
>    release_last_vdef (bb1);
>    same_succ_flush_bb (bb1);
> diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
> index 4199290..5393865 100644
> --- gcc/tree-ssanames.c
> +++ gcc/tree-ssanames.c
> @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "backend.h"
>  #include "tree.h"
>  #include "gimple.h"
> +#include "gimple-iterator.h"
>  #include "hard-reg-set.h"
>  #include "ssa.h"
>  #include "alias.h"
> @@ -544,6 +545,21 @@ reset_flow_sensitive_info (tree name)
>      SSA_NAME_RANGE_INFO (name) = NULL;
>  }
>  
> +/* Clear all flow sensitive data from all statements in BB.  */
> +
> +void
> +reset_flow_sensitive_info_in_bb (basic_block bb)
> +{
> +  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
> +       gsi_next (&gsi))
> +    {
> +      gimple *stmt = gsi_stmt (gsi);
> +      ssa_op_iter i;
> +      tree op;
> +      FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
> +	reset_flow_sensitive_info (op);
> +    }
> +}
>  
>  /* Release all the SSA_NAMEs created by STMT.  */
>  
> diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h
> index 22ff609..5688ca5 100644
> --- gcc/tree-ssanames.h
> +++ gcc/tree-ssanames.h
> @@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple *);
>  extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
>  					   struct range_info_def *);
>  extern void reset_flow_sensitive_info (tree);
> +extern void reset_flow_sensitive_info_in_bb (basic_block);
>  extern void release_defs (gimple *);
>  extern void replace_ssa_name_symbol (tree, tree);
>  
> 
> 	Marek
> 
> 

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

Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Marek Polacek]

On Fri, Sep 25, 2015 at 09:29:30AM +0200, Richard Biener wrote:
> On Thu, 24 Sep 2015, Marek Polacek wrote:
> 
> > As Richi said in <https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
> > using recorded SSA name range infos in VRP is likely to expose errors in the
> > ranges.  This PR is such a case.  As discussed in the PR, after tail merging
> > via PRE the range infos cannot be relied upon anymore, so we need to clear
> > them.
> > 
> > Since tree-ssa-ifcombine.c already had code to clean up the flow data in a BB,
> > I've factored it out to a common function.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?
> 
> I believe for tail-merge you also need to clear range info on
> PHI defs in the BB.  For ifcombine this wasn't necessary (no PHI nodes
> in the relevant CFG), but it's ok to extend the new 
> reset_flow_sensitive_info_in_bb function to also reset PHI defs.

All right.
 
> Ok with that change.

Since I'm not completely sure if I did the right thing here, could you
please have another look at the new function?

Bootstrapped/regtested on x86_64-linux, ok for trunk/5?

2015-09-25  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/67690
	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
	reset_flow_sensitive_info_in_bb.
	* tree-ssa-tail-merge.c (replace_block_by): Likewise.
	* tree-ssanames.c: Include "gimple-iterator.h".
	(reset_flow_sensitive_info_in_bb): New function.
	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.

	* gcc.dg/torture/pr67690.c: New test.

diff --git gcc/testsuite/gcc.dg/torture/pr67690.c gcc/testsuite/gcc.dg/torture/pr67690.c
index e69de29..491de51 100644
--- gcc/testsuite/gcc.dg/torture/pr67690.c
+++ gcc/testsuite/gcc.dg/torture/pr67690.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+const int c1 = 1;
+const int c2 = 2;
+
+int
+check (int i)
+{
+  int j;
+  if (i >= 0)
+    j = c2 - i;
+  else
+    j = c2 - i;
+  return c2 - c1 + 1 > j;
+}
+
+int invoke (int *pi) __attribute__ ((noinline,noclone));
+int
+invoke (int *pi)
+{
+  return check (*pi);
+}
+
+int
+main ()
+{
+  int i = c1;
+  int ret = invoke (&i);
+  if (!ret)
+    __builtin_abort ();
+  return 0;
+}
diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c
index 9f04174..66be430 100644
--- gcc/tree-ssa-ifcombine.c
+++ gcc/tree-ssa-ifcombine.c
@@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun)
 	  {
 	    /* Clear range info from all stmts in BB which is now executed
 	       conditional on a always true/false condition.  */
-	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
-		 !gsi_end_p (gsi); gsi_next (&gsi))
-	      {
-		gimple *stmt = gsi_stmt (gsi);
-		ssa_op_iter i;
-		tree op;
-		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
-		  reset_flow_sensitive_info (op);
-	      }
-
+	    reset_flow_sensitive_info_in_bb (bb);
 	    cfg_changed |= true;
 	  }
     }
diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c
index 0ce59e8..487961e 100644
--- gcc/tree-ssa-tail-merge.c
+++ gcc/tree-ssa-tail-merge.c
@@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block bb2)
       e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
     }
 
+  /* Clear range info from all stmts in BB2 -- this transformation
+     could make them out of date.  */
+  reset_flow_sensitive_info_in_bb (bb2);
+
   /* Do updates that use bb1, before deleting bb1.  */
   release_last_vdef (bb1);
   same_succ_flush_bb (bb1);
diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
index 4199290..ef21137 100644
--- gcc/tree-ssanames.c
+++ gcc/tree-ssanames.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "backend.h"
 #include "tree.h"
 #include "gimple.h"
+#include "gimple-iterator.h"
 #include "hard-reg-set.h"
 #include "ssa.h"
 #include "alias.h"
@@ -544,6 +545,27 @@ reset_flow_sensitive_info (tree name)
     SSA_NAME_RANGE_INFO (name) = NULL;
 }
 
+/* Clear all flow sensitive data from all statements and PHI definitions
+   in BB.  */
+
+void
+reset_flow_sensitive_info_in_bb (basic_block bb)
+{
+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+       gsi_next (&gsi))
+    {
+      gimple *stmt = gsi_stmt (gsi);
+      ssa_op_iter i;
+      def_operand_p def;
+      FOR_EACH_PHI_OR_STMT_DEF (def, stmt, i, SSA_OP_DEF)
+	{
+	  tree var = DEF_FROM_PTR (def);
+	  if (TREE_CODE (var) != SSA_NAME)
+	    continue;
+	  reset_flow_sensitive_info (var);
+	}
+    }
+}
 
 /* Release all the SSA_NAMEs created by STMT.  */
 
diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h
index 22ff609..5688ca5 100644
--- gcc/tree-ssanames.h
+++ gcc/tree-ssanames.h
@@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
 					   struct range_info_def *);
 extern void reset_flow_sensitive_info (tree);
+extern void reset_flow_sensitive_info_in_bb (basic_block);
 extern void release_defs (gimple *);
 extern void replace_ssa_name_symbol (tree, tree);
 

	Marek

Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Marek Polacek]

On Fri, Sep 25, 2015 at 03:49:34PM +0200, Marek Polacek wrote:
> On Fri, Sep 25, 2015 at 09:29:30AM +0200, Richard Biener wrote:
> > On Thu, 24 Sep 2015, Marek Polacek wrote:
> > 
> > > As Richi said in <https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
> > > using recorded SSA name range infos in VRP is likely to expose errors in the
> > > ranges.  This PR is such a case.  As discussed in the PR, after tail merging
> > > via PRE the range infos cannot be relied upon anymore, so we need to clear
> > > them.
> > > 
> > > Since tree-ssa-ifcombine.c already had code to clean up the flow data in a BB,
> > > I've factored it out to a common function.
> > > 
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?
> > 
> > I believe for tail-merge you also need to clear range info on
> > PHI defs in the BB.  For ifcombine this wasn't necessary (no PHI nodes
> > in the relevant CFG), but it's ok to extend the new 
> > reset_flow_sensitive_info_in_bb function to also reset PHI defs.
> 
> All right.
>  
> > Ok with that change.
> 
> Since I'm not completely sure if I did the right thing here, could you
> please have another look at the new function?

And this is gcc-5 variant of the patch, bootstrapped/regtested on x86_64-linux.

2015-09-25  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/67690
	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
	reset_flow_sensitive_info_in_bb.
	* tree-ssa-tail-merge.c: Include "stringpool.h" and "tree-ssanames.h".
	(replace_block_by): Call reset_flow_sensitive_info_in_bb.
	* tree-ssanames.c: Include "gimple-iterator.h".
	(reset_flow_sensitive_info_in_bb): New function.
	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.

	* gcc.dg/torture/pr67690.c: New test.

--- gcc/testsuite/gcc.dg/torture/pr67690.c
+++ gcc/testsuite/gcc.dg/torture/pr67690.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+const int c1 = 1;
+const int c2 = 2;
+
+int
+check (int i)
+{
+  int j;
+  if (i >= 0)
+    j = c2 - i;
+  else
+    j = c2 - i;
+  return c2 - c1 + 1 > j;
+}
+
+int invoke (int *pi) __attribute__ ((noinline,noclone));
+int
+invoke (int *pi)
+{
+  return check (*pi);
+}
+
+int
+main ()
+{
+  int i = c1;
+  int ret = invoke (&i);
+  if (!ret)
+    __builtin_abort ();
+  return 0;
+}
--- gcc/tree-ssa-ifcombine.c
+++ gcc/tree-ssa-ifcombine.c
@@ -806,16 +806,7 @@ pass_tree_ifcombine::execute (function *fun)
 	  {
 	    /* Clear range info from all stmts in BB which is now executed
 	       conditional on a always true/false condition.  */
-	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
-		 !gsi_end_p (gsi); gsi_next (&gsi))
-	      {
-		gimple stmt = gsi_stmt (gsi);
-		ssa_op_iter i;
-		tree op;
-		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
-		  reset_flow_sensitive_info (op);
-	      }
-
+	    reset_flow_sensitive_info_in_bb (bb);
 	    cfg_changed |= true;
 	  }
     }
--- gcc/tree-ssa-tail-merge.c
+++ gcc/tree-ssa-tail-merge.c
@@ -235,6 +235,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgloop.h"
 #include "tree-pass.h"
 #include "trans-mem.h"
+#include "stringpool.h"
+#include "tree-ssanames.h"
 
 /* Describes a group of bbs with the same successors.  The successor bbs are
    cached in succs, and the successor edge flags are cached in succ_flags.
@@ -1558,6 +1560,10 @@ replace_block_by (basic_block bb1, basic_block bb2)
       e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
     }
 
+  /* Clear range info from all stmts in BB2 -- this transformation
+     could make them out of date.  */
+  reset_flow_sensitive_info_in_bb (bb2);
+
   /* Do updates that use bb1, before deleting bb1.  */
   release_last_vdef (bb1);
   same_succ_flush_bb (bb1);
--- gcc/tree-ssanames.c
+++ gcc/tree-ssanames.c
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "is-a.h"
 #include "gimple.h"
 #include "gimple-ssa.h"
+#include "gimple-iterator.h"
 #include "tree-phinodes.h"
 #include "ssa-iterators.h"
 #include "stringpool.h"
@@ -563,6 +564,27 @@ reset_flow_sensitive_info (tree name)
     SSA_NAME_RANGE_INFO (name) = NULL;
 }
 
+/* Clear all flow sensitive data from all statements and PHI definitions
+   in BB.  */
+
+void
+reset_flow_sensitive_info_in_bb (basic_block bb)
+{
+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+       gsi_next (&gsi))
+    {
+      gimple stmt = gsi_stmt (gsi);
+      ssa_op_iter i;
+      def_operand_p def;
+      FOR_EACH_PHI_OR_STMT_DEF (def, stmt, i, SSA_OP_DEF)
+	{
+	  tree var = DEF_FROM_PTR (def);
+	  if (TREE_CODE (var) != SSA_NAME)
+	    continue;
+	  reset_flow_sensitive_info (var);
+	}
+    }
+}
 
 /* Release all the SSA_NAMEs created by STMT.  */
 
--- gcc/tree-ssanames.h
+++ gcc/tree-ssanames.h
@@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple);
 extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
 					   struct range_info_def *);
 extern void reset_flow_sensitive_info (tree);
+extern void reset_flow_sensitive_info_in_bb (basic_block);
 extern void release_defs (gimple);
 extern void replace_ssa_name_symbol (tree, tree);
 

	Marek

Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Richard Biener]

On September 25, 2015 3:49:34 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote:
>On Fri, Sep 25, 2015 at 09:29:30AM +0200, Richard Biener wrote:
>> On Thu, 24 Sep 2015, Marek Polacek wrote:
>> 
>> > As Richi said in
><https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
>> > using recorded SSA name range infos in VRP is likely to expose
>errors in the
>> > ranges.  This PR is such a case.  As discussed in the PR, after
>tail merging
>> > via PRE the range infos cannot be relied upon anymore, so we need
>to clear
>> > them.
>> > 
>> > Since tree-ssa-ifcombine.c already had code to clean up the flow
>data in a BB,
>> > I've factored it out to a common function.
>> > 
>> > Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?
>> 
>> I believe for tail-merge you also need to clear range info on
>> PHI defs in the BB.  For ifcombine this wasn't necessary (no PHI
>nodes
>> in the relevant CFG), but it's ok to extend the new 
>> reset_flow_sensitive_info_in_bb function to also reset PHI defs.
>
>All right.
> 
>> Ok with that change.
>
>Since I'm not completely sure if I did the right thing here, could you
>please have another look at the new function?

Doesn't work that way.  You need to iterate over the PHI sequence separately via gsi_start_phis(bb), etc.

>Bootstrapped/regtested on x86_64-linux, ok for trunk/5?
>
>2015-09-25  Marek Polacek  <polacek@redhat.com>
>
>	PR tree-optimization/67690
>	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
>	reset_flow_sensitive_info_in_bb.
>	* tree-ssa-tail-merge.c (replace_block_by): Likewise.
>	* tree-ssanames.c: Include "gimple-iterator.h".
>	(reset_flow_sensitive_info_in_bb): New function.
>	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.
>
>	* gcc.dg/torture/pr67690.c: New test.
>
>diff --git gcc/testsuite/gcc.dg/torture/pr67690.c
>gcc/testsuite/gcc.dg/torture/pr67690.c
>index e69de29..491de51 100644
>--- gcc/testsuite/gcc.dg/torture/pr67690.c
>+++ gcc/testsuite/gcc.dg/torture/pr67690.c
>@@ -0,0 +1,32 @@
>+/* { dg-do run } */
>+
>+const int c1 = 1;
>+const int c2 = 2;
>+
>+int
>+check (int i)
>+{
>+  int j;
>+  if (i >= 0)
>+    j = c2 - i;
>+  else
>+    j = c2 - i;
>+  return c2 - c1 + 1 > j;
>+}
>+
>+int invoke (int *pi) __attribute__ ((noinline,noclone));
>+int
>+invoke (int *pi)
>+{
>+  return check (*pi);
>+}
>+
>+int
>+main ()
>+{
>+  int i = c1;
>+  int ret = invoke (&i);
>+  if (!ret)
>+    __builtin_abort ();
>+  return 0;
>+}
>diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c
>index 9f04174..66be430 100644
>--- gcc/tree-ssa-ifcombine.c
>+++ gcc/tree-ssa-ifcombine.c
>@@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun)
> 	  {
> 	    /* Clear range info from all stmts in BB which is now executed
> 	       conditional on a always true/false condition.  */
>-	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
>-		 !gsi_end_p (gsi); gsi_next (&gsi))
>-	      {
>-		gimple *stmt = gsi_stmt (gsi);
>-		ssa_op_iter i;
>-		tree op;
>-		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
>-		  reset_flow_sensitive_info (op);
>-	      }
>-
>+	    reset_flow_sensitive_info_in_bb (bb);
> 	    cfg_changed |= true;
> 	  }
>     }
>diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c
>index 0ce59e8..487961e 100644
>--- gcc/tree-ssa-tail-merge.c
>+++ gcc/tree-ssa-tail-merge.c
>@@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block
>bb2)
>       e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
>     }
> 
>+  /* Clear range info from all stmts in BB2 -- this transformation
>+     could make them out of date.  */
>+  reset_flow_sensitive_info_in_bb (bb2);
>+
>   /* Do updates that use bb1, before deleting bb1.  */
>   release_last_vdef (bb1);
>   same_succ_flush_bb (bb1);
>diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
>index 4199290..ef21137 100644
>--- gcc/tree-ssanames.c
>+++ gcc/tree-ssanames.c
>@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
> #include "backend.h"
> #include "tree.h"
> #include "gimple.h"
>+#include "gimple-iterator.h"
> #include "hard-reg-set.h"
> #include "ssa.h"
> #include "alias.h"
>@@ -544,6 +545,27 @@ reset_flow_sensitive_info (tree name)
>     SSA_NAME_RANGE_INFO (name) = NULL;
> }
> 
>+/* Clear all flow sensitive data from all statements and PHI
>definitions
>+   in BB.  */
>+
>+void
>+reset_flow_sensitive_info_in_bb (basic_block bb)
>+{
>+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
>+       gsi_next (&gsi))
>+    {
>+      gimple *stmt = gsi_stmt (gsi);
>+      ssa_op_iter i;
>+      def_operand_p def;
>+      FOR_EACH_PHI_OR_STMT_DEF (def, stmt, i, SSA_OP_DEF)
>+	{
>+	  tree var = DEF_FROM_PTR (def);
>+	  if (TREE_CODE (var) != SSA_NAME)
>+	    continue;
>+	  reset_flow_sensitive_info (var);
>+	}
>+    }
>+}
> 
> /* Release all the SSA_NAMEs created by STMT.  */
> 
>diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h
>index 22ff609..5688ca5 100644
>--- gcc/tree-ssanames.h
>+++ gcc/tree-ssanames.h
>@@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *,
>tree, gimple *);
>extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
> 					   struct range_info_def *);
> extern void reset_flow_sensitive_info (tree);
>+extern void reset_flow_sensitive_info_in_bb (basic_block);
> extern void release_defs (gimple *);
> extern void replace_ssa_name_symbol (tree, tree);
> 
>
>	Marek

Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Marek Polacek]

On Fri, Sep 25, 2015 at 06:22:44PM +0200, Richard Biener wrote:
> On September 25, 2015 3:49:34 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote:
> >On Fri, Sep 25, 2015 at 09:29:30AM +0200, Richard Biener wrote:
> >> On Thu, 24 Sep 2015, Marek Polacek wrote:
> >> 
> >> > As Richi said in
> ><https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
> >> > using recorded SSA name range infos in VRP is likely to expose
> >errors in the
> >> > ranges.  This PR is such a case.  As discussed in the PR, after
> >tail merging
> >> > via PRE the range infos cannot be relied upon anymore, so we need
> >to clear
> >> > them.
> >> > 
> >> > Since tree-ssa-ifcombine.c already had code to clean up the flow
> >data in a BB,
> >> > I've factored it out to a common function.
> >> > 
> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?
> >> 
> >> I believe for tail-merge you also need to clear range info on
> >> PHI defs in the BB.  For ifcombine this wasn't necessary (no PHI
> >nodes
> >> in the relevant CFG), but it's ok to extend the new 
> >> reset_flow_sensitive_info_in_bb function to also reset PHI defs.
> >
> >All right.
> > 
> >> Ok with that change.
> >
> >Since I'm not completely sure if I did the right thing here, could you
> >please have another look at the new function?
> 
> Doesn't work that way.  You need to iterate over the PHI sequence separately via gsi_start_phis(bb), etc.

Oops, sorry.  So like this?

Bootstrapped/regtested on x86_64-linux, ok for trunk (and a similar
patch for 5)?

2015-09-29  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/67690
	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
	reset_flow_sensitive_info_in_bb.
	* tree-ssa-tail-merge.c (replace_block_by): Likewise.
	* tree-ssanames.c: Include "gimple-iterator.h".
	(reset_flow_sensitive_info_in_bb): New function.
	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.

	* gcc.dg/torture/pr67690.c: New test.

diff --git gcc/testsuite/gcc.dg/torture/pr67690.c gcc/testsuite/gcc.dg/torture/pr67690.c
index e69de29..491de51 100644
--- gcc/testsuite/gcc.dg/torture/pr67690.c
+++ gcc/testsuite/gcc.dg/torture/pr67690.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+const int c1 = 1;
+const int c2 = 2;
+
+int
+check (int i)
+{
+  int j;
+  if (i >= 0)
+    j = c2 - i;
+  else
+    j = c2 - i;
+  return c2 - c1 + 1 > j;
+}
+
+int invoke (int *pi) __attribute__ ((noinline,noclone));
+int
+invoke (int *pi)
+{
+  return check (*pi);
+}
+
+int
+main ()
+{
+  int i = c1;
+  int ret = invoke (&i);
+  if (!ret)
+    __builtin_abort ();
+  return 0;
+}
diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c
index 9f04174..66be430 100644
--- gcc/tree-ssa-ifcombine.c
+++ gcc/tree-ssa-ifcombine.c
@@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun)
 	  {
 	    /* Clear range info from all stmts in BB which is now executed
 	       conditional on a always true/false condition.  */
-	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
-		 !gsi_end_p (gsi); gsi_next (&gsi))
-	      {
-		gimple *stmt = gsi_stmt (gsi);
-		ssa_op_iter i;
-		tree op;
-		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
-		  reset_flow_sensitive_info (op);
-	      }
-
+	    reset_flow_sensitive_info_in_bb (bb);
 	    cfg_changed |= true;
 	  }
     }
diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c
index 0ce59e8..487961e 100644
--- gcc/tree-ssa-tail-merge.c
+++ gcc/tree-ssa-tail-merge.c
@@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block bb2)
       e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
     }
 
+  /* Clear range info from all stmts in BB2 -- this transformation
+     could make them out of date.  */
+  reset_flow_sensitive_info_in_bb (bb2);
+
   /* Do updates that use bb1, before deleting bb1.  */
   release_last_vdef (bb1);
   same_succ_flush_bb (bb1);
diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
index 4199290..7235dc3 100644
--- gcc/tree-ssanames.c
+++ gcc/tree-ssanames.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "backend.h"
 #include "tree.h"
 #include "gimple.h"
+#include "gimple-iterator.h"
 #include "hard-reg-set.h"
 #include "ssa.h"
 #include "alias.h"
@@ -544,6 +545,29 @@ reset_flow_sensitive_info (tree name)
     SSA_NAME_RANGE_INFO (name) = NULL;
 }
 
+/* Clear all flow sensitive data from all statements and PHI definitions
+   in BB.  */
+
+void
+reset_flow_sensitive_info_in_bb (basic_block bb)
+{
+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
+       gsi_next (&gsi))
+    {
+      gimple *stmt = gsi_stmt (gsi);
+      ssa_op_iter i;
+      tree op;
+      FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
+	reset_flow_sensitive_info (op);
+    }
+
+  for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
+       gsi_next (&gsi))
+    {
+      tree phi_def = gimple_phi_result (gsi.phi ());
+      reset_flow_sensitive_info (phi_def);
+    }
+}
 
 /* Release all the SSA_NAMEs created by STMT.  */
 
diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h
index 22ff609..5688ca5 100644
--- gcc/tree-ssanames.h
+++ gcc/tree-ssanames.h
@@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *, tree, gimple *);
 extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
 					   struct range_info_def *);
 extern void reset_flow_sensitive_info (tree);
+extern void reset_flow_sensitive_info_in_bb (basic_block);
 extern void release_defs (gimple *);
 extern void replace_ssa_name_symbol (tree, tree);
 

	Marek

Re: [PATCH] Clear variables with stale SSA_NAME_RANGE_INFO (PR tree-optimization/67690)

[Richard Biener]

On September 29, 2015 4:21:16 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote:
>On Fri, Sep 25, 2015 at 06:22:44PM +0200, Richard Biener wrote:
>> On September 25, 2015 3:49:34 PM GMT+02:00, Marek Polacek
><polacek@redhat.com> wrote:
>> >On Fri, Sep 25, 2015 at 09:29:30AM +0200, Richard Biener wrote:
>> >> On Thu, 24 Sep 2015, Marek Polacek wrote:
>> >> 
>> >> > As Richi said in
>> ><https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02279.html>,
>> >> > using recorded SSA name range infos in VRP is likely to expose
>> >errors in the
>> >> > ranges.  This PR is such a case.  As discussed in the PR, after
>> >tail merging
>> >> > via PRE the range infos cannot be relied upon anymore, so we
>need
>> >to clear
>> >> > them.
>> >> > 
>> >> > Since tree-ssa-ifcombine.c already had code to clean up the flow
>> >data in a BB,
>> >> > I've factored it out to a common function.
>> >> > 
>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk and 5?
>> >> 
>> >> I believe for tail-merge you also need to clear range info on
>> >> PHI defs in the BB.  For ifcombine this wasn't necessary (no PHI
>> >nodes
>> >> in the relevant CFG), but it's ok to extend the new 
>> >> reset_flow_sensitive_info_in_bb function to also reset PHI defs.
>> >
>> >All right.
>> > 
>> >> Ok with that change.
>> >
>> >Since I'm not completely sure if I did the right thing here, could
>you
>> >please have another look at the new function?
>> 
>> Doesn't work that way.  You need to iterate over the PHI sequence
>separately via gsi_start_phis(bb), etc.
>
>Oops, sorry.  So like this?
>
>Bootstrapped/regtested on x86_64-linux, ok for trunk (and a similar
>patch for 5)?

Yes, thanks
Richard.

>2015-09-29  Marek Polacek  <polacek@redhat.com>
>
>	PR tree-optimization/67690
>	* tree-ssa-ifcombine.c (pass_tree_ifcombine::execute): Call
>	reset_flow_sensitive_info_in_bb.
>	* tree-ssa-tail-merge.c (replace_block_by): Likewise.
>	* tree-ssanames.c: Include "gimple-iterator.h".
>	(reset_flow_sensitive_info_in_bb): New function.
>	* tree-ssanames.h (reset_flow_sensitive_info_in_bb): Declare.
>
>	* gcc.dg/torture/pr67690.c: New test.
>
>diff --git gcc/testsuite/gcc.dg/torture/pr67690.c
>gcc/testsuite/gcc.dg/torture/pr67690.c
>index e69de29..491de51 100644
>--- gcc/testsuite/gcc.dg/torture/pr67690.c
>+++ gcc/testsuite/gcc.dg/torture/pr67690.c
>@@ -0,0 +1,32 @@
>+/* { dg-do run } */
>+
>+const int c1 = 1;
>+const int c2 = 2;
>+
>+int
>+check (int i)
>+{
>+  int j;
>+  if (i >= 0)
>+    j = c2 - i;
>+  else
>+    j = c2 - i;
>+  return c2 - c1 + 1 > j;
>+}
>+
>+int invoke (int *pi) __attribute__ ((noinline,noclone));
>+int
>+invoke (int *pi)
>+{
>+  return check (*pi);
>+}
>+
>+int
>+main ()
>+{
>+  int i = c1;
>+  int ret = invoke (&i);
>+  if (!ret)
>+    __builtin_abort ();
>+  return 0;
>+}
>diff --git gcc/tree-ssa-ifcombine.c gcc/tree-ssa-ifcombine.c
>index 9f04174..66be430 100644
>--- gcc/tree-ssa-ifcombine.c
>+++ gcc/tree-ssa-ifcombine.c
>@@ -769,16 +769,7 @@ pass_tree_ifcombine::execute (function *fun)
> 	  {
> 	    /* Clear range info from all stmts in BB which is now executed
> 	       conditional on a always true/false condition.  */
>-	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
>-		 !gsi_end_p (gsi); gsi_next (&gsi))
>-	      {
>-		gimple *stmt = gsi_stmt (gsi);
>-		ssa_op_iter i;
>-		tree op;
>-		FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
>-		  reset_flow_sensitive_info (op);
>-	      }
>-
>+	    reset_flow_sensitive_info_in_bb (bb);
> 	    cfg_changed |= true;
> 	  }
>     }
>diff --git gcc/tree-ssa-tail-merge.c gcc/tree-ssa-tail-merge.c
>index 0ce59e8..487961e 100644
>--- gcc/tree-ssa-tail-merge.c
>+++ gcc/tree-ssa-tail-merge.c
>@@ -1534,6 +1534,10 @@ replace_block_by (basic_block bb1, basic_block
>bb2)
>       e2->probability = GCOV_COMPUTE_SCALE (e2->count, out_sum);
>     }
> 
>+  /* Clear range info from all stmts in BB2 -- this transformation
>+     could make them out of date.  */
>+  reset_flow_sensitive_info_in_bb (bb2);
>+
>   /* Do updates that use bb1, before deleting bb1.  */
>   release_last_vdef (bb1);
>   same_succ_flush_bb (bb1);
>diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
>index 4199290..7235dc3 100644
>--- gcc/tree-ssanames.c
>+++ gcc/tree-ssanames.c
>@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
> #include "backend.h"
> #include "tree.h"
> #include "gimple.h"
>+#include "gimple-iterator.h"
> #include "hard-reg-set.h"
> #include "ssa.h"
> #include "alias.h"
>@@ -544,6 +545,29 @@ reset_flow_sensitive_info (tree name)
>     SSA_NAME_RANGE_INFO (name) = NULL;
> }
> 
>+/* Clear all flow sensitive data from all statements and PHI
>definitions
>+   in BB.  */
>+
>+void
>+reset_flow_sensitive_info_in_bb (basic_block bb)
>+{
>+  for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
>+       gsi_next (&gsi))
>+    {
>+      gimple *stmt = gsi_stmt (gsi);
>+      ssa_op_iter i;
>+      tree op;
>+      FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
>+	reset_flow_sensitive_info (op);
>+    }
>+
>+  for (gphi_iterator gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
>+       gsi_next (&gsi))
>+    {
>+      tree phi_def = gimple_phi_result (gsi.phi ());
>+      reset_flow_sensitive_info (phi_def);
>+    }
>+}
> 
> /* Release all the SSA_NAMEs created by STMT.  */
> 
>diff --git gcc/tree-ssanames.h gcc/tree-ssanames.h
>index 22ff609..5688ca5 100644
>--- gcc/tree-ssanames.h
>+++ gcc/tree-ssanames.h
>@@ -95,6 +95,7 @@ extern tree duplicate_ssa_name_fn (struct function *,
>tree, gimple *);
>extern void duplicate_ssa_name_range_info (tree, enum value_range_type,
> 					   struct range_info_def *);
> extern void reset_flow_sensitive_info (tree);
>+extern void reset_flow_sensitive_info_in_bb (basic_block);
> extern void release_defs (gimple *);
> extern void replace_ssa_name_symbol (tree, tree);
> 
>
>	Marek