diff mbox

Reset or update VR during phiopt value_replacement (PR tree-optimization/65053)

Message ID 20150213195201.GP1746@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 13, 2015, 7:52 p.m. UTC 
Hi!

When moving assignment out of a conditional block before the condition, we
need to either reset or update SSA_NAME_RANGE_INFO, otherwise it might be
wrong, because when computed earlier, it could take into account that in
the conditional block the condition is or is not true.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2015-02-13  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/65053
	* tree-ssa-phiopt.c (value_replacement): When moving assign before
	cond, either reset VR on lhs or set it to phi result VR.

	* gcc.c-torture/execute/pr65053-1.c: New test.
	* gcc.c-torture/execute/pr65053-2.c: New test.


	Jakub

Comments

Jeff Law Feb. 13, 2015, 10:51 p.m. UTC | #1 
On 02/13/15 12:52, Jakub Jelinek wrote:
> Hi!
>
> When moving assignment out of a conditional block before the condition, we
> need to either reset or update SSA_NAME_RANGE_INFO, otherwise it might be
> wrong, because when computed earlier, it could take into account that in
> the conditional block the condition is or is not true.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2015-02-13  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/65053
> 	* tree-ssa-phiopt.c (value_replacement): When moving assign before
> 	cond, either reset VR on lhs or set it to phi result VR.
>
> 	* gcc.c-torture/execute/pr65053-1.c: New test.
> 	* gcc.c-torture/execute/pr65053-2.c: New test.
OK.

I wouldn't be terribly surprised if we have a variety of these kind of 
bugs lurking.  Basically everything which moves statements like this 
potentially needs updating.

jeff
diff mbox

Patch

--- gcc/tree-ssa-phiopt.c.jj	2015-01-15 20:25:39.000000000 +0100
+++ gcc/tree-ssa-phiopt.c	2015-02-13 18:19:39.990627277 +0100
@@ -917,6 +917,31 @@  value_replacement (basic_block cond_bb,
 	      && absorbing_element_p (code_def, cond_rhs))))
     {
       gsi = gsi_for_stmt (cond);
+      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+	{
+	  /* Moving ASSIGN might change VR of lhs, e.g. when moving u_6
+	     def-stmt in:
+	     if (n_5 != 0)
+	       goto <bb 3>;
+	     else
+	       goto <bb 4>;
+
+	     <bb 3>:
+	     # RANGE [0, 4294967294]
+	     u_6 = n_5 + 4294967295;
+
+	     <bb 4>:
+	     # u_3 = PHI <u_6(3), 4294967295(2)>  */
+	  SSA_NAME_RANGE_INFO (lhs) = NULL;
+	  SSA_NAME_ANTI_RANGE_P (lhs) = 0;
+	  /* If available, we can use VR of phi result at least.  */
+	  tree phires = gimple_phi_result (phi);
+	  struct range_info_def *phires_range_info
+	    = SSA_NAME_RANGE_INFO (phires);
+	  if (phires_range_info)
+	    duplicate_ssa_name_range_info (lhs, SSA_NAME_RANGE_TYPE (phires),
+					   phires_range_info);
+	}
       gimple_stmt_iterator gsi_from = gsi_for_stmt (assign);
       gsi_move_before (&gsi_from, &gsi);
       replace_phi_edge_with_variable (cond_bb, e1, phi, lhs);
--- gcc/testsuite/gcc.c-torture/execute/pr65053-1.c.jj	2015-02-13 18:45:40.149071898 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr65053-1.c	2015-02-13 18:40:53.000000000 +0100
@@ -0,0 +1,32 @@ 
+/* PR tree-optimization/65053 */
+
+int i;
+
+__attribute__ ((noinline, noclone))
+unsigned int foo (void)
+{
+  return 0;
+}
+
+int
+main ()
+{
+  unsigned int u = -1;
+  if (u == -1)
+    {
+      unsigned int n = foo ();
+      if (n > 0)
+	u = n - 1;
+    }
+
+  while (u != -1)
+    {
+      asm ("" : "+g" (u));
+      u = -1;
+      i = 1;
+    }
+
+  if (i)
+    __builtin_abort ();
+  return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr65053-2.c.jj	2015-02-13 18:45:43.552016154 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr65053-2.c	2015-02-13 18:44:59.000000000 +0100
@@ -0,0 +1,27 @@ 
+/* PR tree-optimization/65053 */
+
+int i;
+unsigned int x;
+
+int
+main ()
+{
+  asm volatile ("" : "+g" (x));
+  unsigned int n = x;
+  unsigned int u = 32;
+  if (n >= 32)
+    __builtin_abort ();
+  if (n != 0)
+    u = n + 32;
+
+  while (u != 32)
+    {
+      asm ("" : : "g" (u));
+      u = 32;
+      i = 1;
+    }
+
+  if (i)
+    __builtin_abort ();
+  return 0;
+}