Re: [PATCH] Fix reassociation not to reuse SSA_NAMEs (PR tree-optimization/71631)

[Richard Biener <rguenther at suse dot de>]

On June 24, 2016 8:30:16 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>rewrite_expr_tree tries to reuse SSA_NAMEs if it knows they hold the
>same
>values, which it figures out by setting the changed flag during
>recursion
>if some operand is different.  But in order for this to work reliably,
>we need to be started with the right changed value in the outermost
>call
>- we need it to be clear only if the result of rewrite_expr_tree is the
>original lhs, i.e. we don't add any further operations afterwards.
>This used to be correct before the introduction of negate_result stuff,
>i.e. it was clear unless powi_result != NULL.  But negate_result
>needs to be treated the same, and we need to be prepared for
>rewrite_expr_tree returning in that case something other than lhs.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

>2016-06-24  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/71631
>	* tree-ssa-reassoc.c (reassociate_bb): Pass true as last argument
>	to rewrite_expr_tree even if negate_result, move new_lhs var
>	declaration and initialization earlier, for powi_result set afterwards
>	new_lhs to lhs.  For negate_result, use new_lhs instead of tmp
>	if new_lhs != lhs, and don't shadow gsi var.
>
>	* gcc.c-torture/execute/pr71631.c: New test.
>
>--- gcc/tree-ssa-reassoc.c.jj	2016-06-24 12:59:05.632625218 +0200
>+++ gcc/tree-ssa-reassoc.c	2016-06-24 17:30:20.493848465 +0200
>@@ -5314,6 +5314,7 @@ reassociate_bb (basic_block bb)
> 		    }
> 		}
> 
>+	      tree new_lhs = lhs;
> 	      /* If the operand vector is now empty, all operands were 
> 		 consumed by the __builtin_powi optimization.  */
> 	      if (ops.length () == 0)
>@@ -5337,7 +5338,6 @@ reassociate_bb (basic_block bb)
> 		  machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
> 		  int ops_num = ops.length ();
> 		  int width = get_reassociation_width (ops_num, rhs_code, mode);
>-		  tree new_lhs = lhs;
> 
> 		  if (dump_file && (dump_flags & TDF_DETAILS))
> 		    fprintf (dump_file,
>@@ -5357,7 +5357,8 @@ reassociate_bb (basic_block bb)
>                         swap_ops_for_binary_stmt (ops, len - 3, stmt);
> 
> 		      new_lhs = rewrite_expr_tree (stmt, 0, ops,
>-						   powi_result != NULL);
>+						   powi_result != NULL
>+						   || negate_result);
>                     }
> 
> 		  /* If we combined some repeated factors into a 
>@@ -5372,7 +5373,10 @@ reassociate_bb (basic_block bb)
> 		      gimple_set_lhs (lhs_stmt, target_ssa);
> 		      update_stmt (lhs_stmt);
> 		      if (lhs != new_lhs)
>-			target_ssa = new_lhs;
>+			{
>+			  target_ssa = new_lhs;
>+			  new_lhs = lhs;
>+			}
> 		      mul_stmt = gimple_build_assign (lhs, MULT_EXPR,
> 						      powi_result, target_ssa);
> 		      gimple_set_location (mul_stmt, gimple_location (stmt));
>@@ -5386,10 +5390,11 @@ reassociate_bb (basic_block bb)
> 		  stmt = SSA_NAME_DEF_STMT (lhs);
> 		  tree tmp = make_ssa_name (TREE_TYPE (lhs));
> 		  gimple_set_lhs (stmt, tmp);
>+		  if (lhs != new_lhs)
>+		    tmp = new_lhs;
> 		  gassign *neg_stmt = gimple_build_assign (lhs, NEGATE_EXPR,
> 							   tmp);
> 		  gimple_set_uid (neg_stmt, gimple_uid (stmt));
>-		  gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> 		  gsi_insert_after (&gsi, neg_stmt, GSI_NEW_STMT);
> 		  update_stmt (stmt);
> 		}
>--- gcc/testsuite/gcc.c-torture/execute/pr71631.c.jj	2016-06-24
>14:58:59.185809799 +0200
>+++ gcc/testsuite/gcc.c-torture/execute/pr71631.c	2016-06-24
>14:58:59.185809799 +0200
>@@ -0,0 +1,32 @@
>+/* PR tree-optimization/71631 */
>+
>+volatile char v;
>+int a = 1, b = 1, c = 1;
>+
>+void
>+foo (const char *s)
>+{
>+  while (*s++)
>+    v = *s;
>+}
>+
>+int
>+main ()
>+{
>+  volatile int d = 1;
>+  volatile int e = 1;
>+  int f = 1 / a;
>+  int g = 1U < f;
>+  int h = 2 + g;
>+  int i = 3 % h;
>+  int j = e && b;
>+  int k = 1 == c;
>+  int l = d != 0;
>+  short m = (short) (-1 * i * l);
>+  short x = j * (k * m);
>+  if (i == 1)
>+    foo ("AB");
>+  if (x != -1)
>+    __builtin_abort ();
>+  return 0;
>+}
>
>	Jakub