On June 24, 2016 8:30:16 PM GMT+02:00, Jakub Jelinek <ja...@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 <ja...@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
