On Sat, Apr 23, 2016 at 3:10 PM, kugan <kugan.vivekanandara...@linaro.org> wrote: > >>> I am not sure I understand this. I tried doing this. If I add -1 and >>> rhs1 >>> for the NEGATE_EXPR to ops list, when it come to rewrite_expr_tree >>> constant >>> will be sorted early and would make it hard to generate: >>> x + (-y * z * z) => x - y * z * z >>> >>> Do you want to swap the constant in MULT_EXPR chain (if present) like in >>> swap_ops_for_binary_stmt and then create a NEGATE_EXPR ? >> >> >> In addition to linearize_expr handling you need to handle a -1 in the >> MULT_EXPR >> chain specially at rewrite_expr_tree time by emitting a NEGATE_EXPR >> instead >> of a MULT_EXPR (which also means you should sort the -1 "last"). > > > Hi Richard, > > Thanks. Here is an attempt which does this. > > Regression tested and bootstrapped on x86-64-linux-gnu with no new > regressions. > > Is this OK for trunk?
+ int last = ops.length () - 1; + bool negate_result = false; Do oe &last = ops.last (); + if (rhs_code == MULT_EXPR + && ops.length () > 1 + && ((TREE_CODE (ops[last]->op) == INTEGER_CST and last.op here and below + && integer_minus_onep (ops[last]->op)) + || ((TREE_CODE (ops[last]->op) == REAL_CST) + && real_equal (&TREE_REAL_CST (ops[last]->op), &dconstm1)))) Here the checks !HONOR_SNANS () && (!HONOS_SIGNED_ZEROS || !COMPLEX_FLOAT_TYPE_P) are missing. The * -1 might appear literally and you are only allowed to turn it into a negate under the above conditions. + { + ops.unordered_remove (last); use ops.pop (); + negate_result = true; Please move the whole thing under the else { } case of the ops.length == 0, ops.length == 1 test chain as you did for the actual emit of the negate. + if (negate_result) + { + tree tmp = make_ssa_name (TREE_TYPE (lhs)); + gimple_set_lhs (stmt, tmp); + gassign *neg_stmt = gimple_build_assign (lhs, NEGATE_EXPR, + tmp); + gimple_stmt_iterator gsi = gsi_for_stmt (stmt); + gsi_insert_after (&gsi, neg_stmt, GSI_NEW_STMT); + update_stmt (stmt); I think that if powi_result is also built you end up using the wrong stmt so you miss a stmt = SSA_NAME_DEF_STMT (lhs); here. Also see the new_lhs handling of the powi_result case - again you need sth similar here (it's handling looks a bit fishy as well - this all needs some comments and possibly a (lot more) testcases). So, please do the above requested changes and verify the 'lhs' issues I pointed out by trying to add a few more testcase that also cover the case where a powi is detected in addition to a negation. Please also add a testcase that catches (-y) * x * (-z). Otherwise this now looks good. Thanks, Richard. > Thanks, > Kugan > > 2016-04-23 Kugan Vivekanandarajah <kug...@linaro.org> > > PR middle-end/40921 > * gcc.dg/tree-ssa/pr40921.c: New test. > > gcc/ChangeLog: > > 2016-04-23 Kugan Vivekanandarajah <kug...@linaro.org> > > PR middle-end/40921 > * tree-ssa-reassoc.c (try_special_add_to_ops): New. > (linearize_expr_tree): Call try_special_add_to_ops. > (reassociate_bb): Convert MULT_EXPR by (-1) to NEGATE_EXPR. >