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

Marek Polacek polacek@redhat.com
Tue Sep 29 15:15:00 GMT 2015


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



More information about the Gcc-patches mailing list