Hi Jakub,
On 26 May 2016 at 18:18, Jakub Jelinek <ja...@redhat.com> wrote:
> On Thu, May 26, 2016 at 02:17:56PM +1000, Kugan Vivekanandarajah wrote:
>> --- a/gcc/tree-ssa-reassoc.c
>> +++ b/gcc/tree-ssa-reassoc.c
>> @@ -3767,8 +3767,10 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
>> operand_entry temp = *oe3;
>> oe3->op = oe1->op;
>> oe3->rank = oe1->rank;
>> + oe3->stmt_to_insert = oe1->stmt_to_insert;
>> oe1->op = temp.op;
>> oe1->rank= temp.rank;
>> + oe1->stmt_to_insert = temp.stmt_to_insert;
>
> If you want to swap those 3 fields (what about the others?), can't you write
> std::swap (oe1->op, oe3->op);
> std::swap (oe1->rank, oe3->rank);
> std::swap (oe1->stmt_to_insert, oe3->stmt_to_insert);
> instead and drop operand_entry temp = *oe3; ?
>
>> }
>> else if ((oe1->rank == oe3->rank
>> && oe2->rank != oe3->rank)
>> @@ -3779,8 +3781,10 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
>> operand_entry temp = *oe2;
>> oe2->op = oe1->op;
>> oe2->rank = oe1->rank;
>> + oe2->stmt_to_insert = oe1->stmt_to_insert;
>> oe1->op = temp.op;
>> oe1->rank = temp.rank;
>> + oe1->stmt_to_insert = temp.stmt_to_insert;
>> }
>
> Similarly.
Done. Revised patch attached.
Thanks,
Kugan
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
index e69de29..4dceaaa 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr71269.c
@@ -0,0 +1,10 @@
+/* PR middle-end/71269 */
+/* { dg-do compile } */
+/* { dg-options "-O1" } */
+
+int a, b, c;
+void fn2 (int);
+void fn1 ()
+{
+ fn2 (sizeof 0 + c + a + b + b);
+}
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index c9ed679..db6ac6b 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -3764,11 +3764,9 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
&& !is_phi_for_stmt (stmt, oe1->op)
&& !is_phi_for_stmt (stmt, oe2->op)))
{
- operand_entry temp = *oe3;
- oe3->op = oe1->op;
- oe3->rank = oe1->rank;
- oe1->op = temp.op;
- oe1->rank= temp.rank;
+ std::swap (oe1->op, oe3->op);
+ std::swap (oe1->rank, oe3->rank);
+ std::swap (oe1->stmt_to_insert, oe3->stmt_to_insert);
}
else if ((oe1->rank == oe3->rank
&& oe2->rank != oe3->rank)
@@ -3776,11 +3774,9 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
&& !is_phi_for_stmt (stmt, oe1->op)
&& !is_phi_for_stmt (stmt, oe3->op)))
{
- operand_entry temp = *oe2;
- oe2->op = oe1->op;
- oe2->rank = oe1->rank;
- oe1->op = temp.op;
- oe1->rank = temp.rank;
+ std::swap (oe1->op, oe2->op);
+ std::swap (oe1->rank, oe2->rank);
+ std::swap (oe1->stmt_to_insert, oe2->stmt_to_insert);
}
}
@@ -3790,6 +3786,42 @@ swap_ops_for_binary_stmt (vec<operand_entry *> ops,
static inline gimple *
find_insert_point (gimple *stmt, tree rhs1, tree rhs2)
{
+ /* If rhs1 is defined by stmt_to_insert, insert after its argument
+ definion stmt. */
+ if (TREE_CODE (rhs1) == SSA_NAME
+ && !gimple_nop_p (SSA_NAME_DEF_STMT (rhs1))
+ && !gimple_bb (SSA_NAME_DEF_STMT (rhs1)))
+ {
+ gimple *stmt1 = SSA_NAME_DEF_STMT (rhs1);
+ gcc_assert (is_gimple_assign (stmt1));
+ tree rhs11 = gimple_assign_rhs1 (stmt1);
+ tree rhs12 = gimple_assign_rhs2 (stmt1);
+ if (TREE_CODE (rhs11) == SSA_NAME
+ && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs11)))
+ stmt = SSA_NAME_DEF_STMT (rhs11);
+ if (TREE_CODE (rhs12) == SSA_NAME
+ && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs12)))
+ stmt = SSA_NAME_DEF_STMT (rhs12);
+ }
+
+ /* If rhs2 is defined by stmt_to_insert, insert after its argument
+ definion stmt. */
+ if (TREE_CODE (rhs2) == SSA_NAME
+ && !gimple_nop_p (SSA_NAME_DEF_STMT (rhs2))
+ && !gimple_bb (SSA_NAME_DEF_STMT (rhs2)))
+ {
+ gimple *stmt1 = SSA_NAME_DEF_STMT (rhs2);
+ gcc_assert (is_gimple_assign (stmt1));
+ tree rhs11 = gimple_assign_rhs1 (stmt1);
+ tree rhs12 = gimple_assign_rhs2 (stmt1);
+ if (TREE_CODE (rhs11) == SSA_NAME
+ && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs11)))
+ stmt = SSA_NAME_DEF_STMT (rhs11);
+ if (TREE_CODE (rhs12) == SSA_NAME
+ && reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs12)))
+ stmt = SSA_NAME_DEF_STMT (rhs12);
+ }
+
if (TREE_CODE (rhs1) == SSA_NAME
&& reassoc_stmt_dominates_stmt_p (stmt, SSA_NAME_DEF_STMT (rhs1)))
stmt = SSA_NAME_DEF_STMT (rhs1);
@@ -3843,12 +3875,6 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
{
gimple *insert_point
= find_insert_point (stmt, oe1->op, oe2->op);
- /* If the stmt that defines operand has to be inserted, insert it
- before the use. */
- if (oe1->stmt_to_insert)
- insert_stmt_before_use (stmt, oe1->stmt_to_insert);
- if (oe2->stmt_to_insert)
- insert_stmt_before_use (stmt, oe2->stmt_to_insert);
lhs = make_ssa_name (TREE_TYPE (lhs));
stmt
= gimple_build_assign (lhs, gimple_assign_rhs_code (stmt),
@@ -3864,17 +3890,17 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
{
gcc_checking_assert (find_insert_point (stmt, oe1->op, oe2->op)
== stmt);
- /* If the stmt that defines operand has to be inserted, insert it
- before the use. */
- if (oe1->stmt_to_insert)
- insert_stmt_before_use (stmt, oe1->stmt_to_insert);
- if (oe2->stmt_to_insert)
- insert_stmt_before_use (stmt, oe2->stmt_to_insert);
gimple_assign_set_rhs1 (stmt, oe1->op);
gimple_assign_set_rhs2 (stmt, oe2->op);
update_stmt (stmt);
}
+ /* If the stmt that defines operand has to be inserted, insert it
+ before the use after the stmt is inserted. */
+ if (oe1->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe1->stmt_to_insert);
+ if (oe2->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe2->stmt_to_insert);
if (rhs1 != oe1->op && rhs1 != oe2->op)
remove_visited_stmt_chain (rhs1);
@@ -3893,11 +3919,6 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
/* Rewrite the next operator. */
oe = ops[opindex];
- /* If the stmt that defines operand has to be inserted, insert it
- before the use. */
- if (oe->stmt_to_insert)
- insert_stmt_before_use (stmt, oe->stmt_to_insert);
-
/* Recurse on the LHS of the binary operator, which is guaranteed to
be the non-leaf side. */
tree new_rhs1
@@ -3944,6 +3965,10 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
update_stmt (stmt);
}
+ /* If the stmt that defines operand has to be inserted, insert it
+ before the use after the use_stmt is inserted. */
+ if (oe->stmt_to_insert)
+ insert_stmt_before_use (stmt, oe->stmt_to_insert);
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, " into ");
@@ -4115,24 +4140,41 @@ rewrite_expr_tree_parallel (gassign *stmt, int width,
{
/* If the stmt that defines operand has to be inserted, insert it
before the use. */
- if (stmt1)
- insert_stmt_before_use (stmts[i], stmt1);
- if (stmt2)
- insert_stmt_before_use (stmts[i], stmt2);
gimple_assign_set_rhs1 (stmts[i], op1);
gimple_assign_set_rhs2 (stmts[i], op2);
update_stmt (stmts[i]);
}
else
{
+ /* PR71252: stmt_to_insert has to be inserted after use stmt created
+ by build_and_add_sum. However if the other operand doesnt have
define-stmt
+ or is defined by GIMPLE_NOP, we have to insert it first. */
+ if (stmt1
+ && (TREE_CODE (op2) != SSA_NAME
+ || !gimple_bb (SSA_NAME_DEF_STMT (op2))
+ || gimple_nop_p (SSA_NAME_DEF_STMT (op2))))
+ {
+ insert_stmt_before_use (stmts[i], stmt1);
+ stmt1 = NULL;
+ }
+ if (stmt2
+ && (TREE_CODE (op1) != SSA_NAME
+ || !gimple_bb (SSA_NAME_DEF_STMT (op1))
+ || gimple_nop_p (SSA_NAME_DEF_STMT (op1))))
+ {
+ insert_stmt_before_use (stmts[i], stmt2);
+ stmt2 = NULL;
+ }
stmts[i] = build_and_add_sum (TREE_TYPE (last_rhs1), op1, op2,
opcode);
- /* If the stmt that defines operand has to be inserted, insert it
- before new build_and_add stmt after it is created. */
- if (stmt1)
- insert_stmt_before_use (stmts[i], stmt1);
- if (stmt2)
- insert_stmt_before_use (stmts[i], stmt2);
}
+
+ /* If the stmt that defines operand has to be inserted, insert it
+ before new use stmt after it is created. */
+ if (stmt1)
+ insert_stmt_before_use (stmts[i], stmt1);
+ if (stmt2)
+ insert_stmt_before_use (stmts[i], stmt2);
+ stmt1 = stmt2 = NULL;
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, " into ");
@@ -5312,15 +5354,15 @@ reassociate_bb (basic_block bb)
{
tree last_op = ops.last ()->op;
- /* If the stmt that defines operand has to be inserted,
insert it
- before the use. */
- if (ops.last ()->stmt_to_insert)
- insert_stmt_before_use (stmt, ops.last ()->stmt_to_insert);
if (powi_result)
transform_stmt_to_multiply (&gsi, stmt, last_op,
powi_result);
else
transform_stmt_to_copy (&gsi, stmt, last_op);
+ /* If the stmt that defines operand has to be inserted,
insert it
+ before the use. */
+ if (ops.last ()->stmt_to_insert)
+ insert_stmt_before_use (stmt, ops.last ()->stmt_to_insert);
}
else
{
