Err. I think the way you implement that in reassoc is ad-hoc and not
related to reassoc at all.
In fact what reassoc is missing is to handle
-y * z * (-w) * x -> y * x * w * x
thus optimize negates as if they were additional * -1 entries in a
multiplication chain. And
then optimize a single remaining * -1 in the result chain to a negate.
Then match.pd handles x + (-y) -> x - y (independent of -frounding-math btw).
So no, this isn't ok as-is, IMHO you want to expand the multiplication ops chain
pulling in the * -1 ops (if single-use, of course).
I agree. Here is the updated patch along what you suggested. Does this
look better ?
Thanks,
Kugan
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 17eb64f..bbb5ffb 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -4674,6 +4674,41 @@ attempt_builtin_powi (gimple *stmt, vec<operand_entry *>
*ops)
return result;
}
+/* Factor out NEGATE_EXPR from the multiplication operands. */
+static void
+factor_out_negate_expr (gimple_stmt_iterator *gsi,
+ gimple *stmt, vec<operand_entry *> *ops)
+{
+ operand_entry *oe;
+ unsigned int i;
+ int neg_count = 0;
+
+ FOR_EACH_VEC_ELT (*ops, i, oe)
+ {
+ if (TREE_CODE (oe->op) != SSA_NAME
+ || !has_single_use (oe->op))
+ continue;
+ gimple *def_stmt = SSA_NAME_DEF_STMT (oe->op);
+ if (!is_gimple_assign (def_stmt)
+ || gimple_assign_rhs_code (def_stmt) != NEGATE_EXPR)
+ continue;
+ oe->op = gimple_assign_rhs1 (def_stmt);
+ neg_count ++;
+ }
+
+ if (neg_count % 2)
+ {
+ tree lhs = gimple_assign_lhs (stmt);
+ tree tmp = make_temp_ssa_name (TREE_TYPE (lhs), NULL, "reassocneg");
+ gimple_set_lhs (stmt, tmp);
+ gassign *neg_stmt = gimple_build_assign (lhs, NEGATE_EXPR,
+ tmp);
+ gimple_set_location (neg_stmt, gimple_location (stmt));
+ gimple_set_uid (neg_stmt, gimple_uid (stmt));
+ gsi_insert_after (gsi, neg_stmt, GSI_SAME_STMT);
+ }
+}
+
/* Attempt to optimize
CST1 * copysign (CST2, y) -> copysign (CST1 * CST2, y) if CST1 > 0, or
CST1 * copysign (CST2, y) -> -copysign (CST1 * CST2, y) if CST1 < 0. */
@@ -4917,6 +4952,12 @@ reassociate_bb (basic_block bb)
if (rhs_code == MULT_EXPR)
attempt_builtin_copysign (&ops);
+ if (rhs_code == MULT_EXPR)
+ {
+ factor_out_negate_expr (&gsi, stmt, &ops);
+ ops.qsort (sort_by_operand_rank);
+ }
+
if (reassoc_insert_powi_p
&& rhs_code == MULT_EXPR
&& flag_unsafe_math_optimizations)
