Hi Richard,
> I think it should have the same rank as op or op + 1 which is the current
> behavior. Sth else doesn't work correctly here I think, like inserting the
> multiplication not near the definition of op.
>
> Well, the whole "clever insertion" logic is simply flawed.
What I meant to say was that the simple logic we have now wouldn’t
work. "clever logic" is knowing where exactly where it is needed and
inserting there. I think thats what you are suggesting below in a
simple to implement way.
> I'd say that ideally we would delay inserting the multiplication to
> rewrite_expr_tree time. For example by adding a ops->stmt_to_insert
> member.
>
Here is an implementation based on above. Bootstrap on x86-linux-gnu
is OK. regression testing is ongoing.
Thanks,
Kugan
gcc/ChangeLog:
2016-05-20 Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org>
* tree-ssa-reassoc.c (struct operand_entry): Add field stmt_to_insert.
(add_to_ops_vec): Add stmt_to_insert.
(add_repeat_to_ops_vec): Init stmt_to_insert.
(transform_add_to_multiply): Remove mult_stmt insertion and add it
to ops vector.
(get_ops): Init stmt_to_insert.
(maybe_optimize_range_tests): Likewise.
(rewrite_expr_tree): Insert stmt_to_insert before use stmt.
(rewrite_expr_tree_parallel): Likewise.
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 3b5f36b..69441ce 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -195,6 +195,7 @@ struct operand_entry
int id;
tree op;
unsigned int count;
+ gimple *stmt_to_insert;
};
static object_allocator<operand_entry> operand_entry_pool
@@ -553,7 +554,7 @@ sort_by_operand_rank (const void *pa, const void *pb)
/* Add an operand entry to *OPS for the tree operand OP. */
static void
-add_to_ops_vec (vec<operand_entry *> *ops, tree op)
+add_to_ops_vec (vec<operand_entry *> *ops, tree op, gimple *stmt_to_insert =
NULL)
{
operand_entry *oe = operand_entry_pool.allocate ();
@@ -561,6 +562,7 @@ add_to_ops_vec (vec<operand_entry *> *ops, tree op)
oe->rank = get_rank (op);
oe->id = next_operand_entry_id++;
oe->count = 1;
+ oe->stmt_to_insert = stmt_to_insert;
ops->safe_push (oe);
}
@@ -577,6 +579,7 @@ add_repeat_to_ops_vec (vec<operand_entry *> *ops, tree op,
oe->rank = get_rank (op);
oe->id = next_operand_entry_id++;
oe->count = repeat;
+ oe->stmt_to_insert = NULL;
ops->safe_push (oe);
reassociate_stats.pows_encountered++;
@@ -1810,21 +1813,12 @@ transform_add_to_multiply (gimple *stmt,
vec<operand_entry *> *ops)
ops->unordered_remove (i);
tree tmp = make_ssa_name (TREE_TYPE (op));
tree cst = build_int_cst (integer_type_node, count);
- gimple *def_stmt = SSA_NAME_DEF_STMT (op);
gassign *mul_stmt
= gimple_build_assign (tmp, MULT_EXPR,
op, fold_convert (TREE_TYPE (op), cst));
- if (gimple_code (def_stmt) == GIMPLE_NOP
- || gimple_bb (stmt) != gimple_bb (def_stmt))
- {
- gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
- gimple_set_uid (mul_stmt, gimple_uid (stmt));
- gsi_insert_before (&gsi, mul_stmt, GSI_NEW_STMT);
- }
- else
- insert_stmt_after (mul_stmt, def_stmt);
+ gimple_set_uid (mul_stmt, gimple_uid (stmt));
gimple_set_visited (mul_stmt, true);
- add_to_ops_vec (ops, tmp);
+ add_to_ops_vec (ops, tmp, mul_stmt);
changed = true;
}
@@ -3224,6 +3218,7 @@ get_ops (tree var, enum tree_code code, vec<operand_entry
*> *ops,
oe->rank = code;
oe->id = 0;
oe->count = 1;
+ oe->stmt_to_insert = NULL;
ops->safe_push (oe);
}
return true;
@@ -3464,6 +3459,7 @@ maybe_optimize_range_tests (gimple *stmt)
oe->rank = code;
oe->id = 0;
oe->count = 1;
+ oe->stmt_to_insert = NULL;
ops.safe_push (oe);
bb_ent.last_idx++;
}
@@ -3501,6 +3497,7 @@ maybe_optimize_range_tests (gimple *stmt)
is. */
oe->id = bb->index;
oe->count = 1;
+ oe->stmt_to_insert = NULL;
ops.safe_push (oe);
bb_ent.op = NULL;
bb_ent.last_idx++;
@@ -3798,6 +3795,19 @@ rewrite_expr_tree (gimple *stmt, unsigned int opindex,
oe1 = ops[opindex];
oe2 = ops[opindex + 1];
+ /* If the stmt that defines operand has to be inserted, insert it
+ before the use. */
+ if (oe1->stmt_to_insert)
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gsi_insert_before (&gsi, oe1->stmt_to_insert, GSI_NEW_STMT);
+ }
+ if (oe2->stmt_to_insert)
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gsi_insert_before (&gsi, oe2->stmt_to_insert, GSI_NEW_STMT);
+ }
+
if (rhs1 != oe1->op || rhs2 != oe2->op)
{
gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
@@ -3855,6 +3865,14 @@ 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)
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gsi_insert_before (&gsi, oe->stmt_to_insert, GSI_NEW_STMT);
+ }
+
/* Recurse on the LHS of the binary operator, which is guaranteed to
be the non-leaf side. */
tree new_rhs1
@@ -3999,6 +4017,7 @@ rewrite_expr_tree_parallel (gassign *stmt, int width,
int stmt_index = 0;
int ready_stmts_end = 0;
int i = 0;
+ gimple *stmt1 = NULL, *stmt2 = NULL;
tree last_rhs1 = gimple_assign_rhs1 (stmt);
/* We start expression rewriting from the top statements.
@@ -4027,7 +4046,11 @@ rewrite_expr_tree_parallel (gassign *stmt, int width,
if (ready_stmts_end > stmt_index)
op2 = gimple_assign_lhs (stmts[stmt_index++]);
else if (op_index >= 0)
- op2 = ops[op_index--]->op;
+ {
+ operand_entry *oe = ops[op_index--];
+ stmt2 = oe->stmt_to_insert;
+ op2 = oe->op;
+ }
else
{
gcc_assert (stmt_index < i);
@@ -4041,8 +4064,12 @@ rewrite_expr_tree_parallel (gassign *stmt, int width,
{
if (op_index > 1)
swap_ops_for_binary_stmt (ops, op_index - 2, NULL);
- op2 = ops[op_index--]->op;
- op1 = ops[op_index--]->op;
+ operand_entry *oe2 = ops[op_index--];
+ operand_entry *oe1 = ops[op_index--];
+ op2 = oe2->op;
+ stmt2 = oe2->stmt_to_insert;
+ op1 = oe1->op;
+ stmt1 = oe1->stmt_to_insert;
}
/* If we emit the last statement then we should put
@@ -4057,6 +4084,19 @@ rewrite_expr_tree_parallel (gassign *stmt, int width,
print_gimple_stmt (dump_file, stmts[i], 0, 0);
}
+ /* If the stmt that defines operand has to be inserted, insert it
+ before the use. */
+ if (stmt1)
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmts[i]);
+ gsi_insert_before (&gsi, stmt1, GSI_NEW_STMT);
+ }
+ if (stmt2)
+ {
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmts[i]);
+ gsi_insert_before (&gsi, stmt2, GSI_NEW_STMT);
+ }
+
/* We keep original statement only for the last one. All
others are recreated. */
if (i == stmt_num - 1)
