Hi Richard,

On 19/04/16 22:11, Richard Biener wrote:
On Tue, Apr 19, 2016 at 1:36 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
On Tue, Apr 19, 2016 at 1:35 PM, Richard Biener
<richard.guent...@gmail.com> wrote:
On Mon, Feb 29, 2016 at 11:53 AM, kugan
<kugan.vivekanandara...@linaro.org> wrote:

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 ?

It looks better but I think you want to do factor_out_negate_expr before the
first qsort/optimize_ops_list call to catch -1. * z * (-w) which also means you
want to simply append a -1. to the ops list rather than adjusting the result
with a negate stmt.

You also need to guard all this with ! HONOR_SNANS (type) && (!
HONOR_SIGNED_ZEROS (type)
|| ! COMPLEX_FLOAT_TYPE_P (type)) (see match.pd pattern transforming x
* -1. to -x).

And please add at least one testcase.

And it appears to me that you could handle this in linearize_expr_tree
as well, similar
to how we handle MULT_EXPR with acceptable_pow_call there by adding -1. and
op into the ops vec.



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 ?


Thanks,
Kugan

Similar for the x + x + x -> 3 * x case we'd want to add a repeat op when seeing
x + 3 * x + x and use ->count in that patch as well.

Best split out the

   if (rhscode == MULT_EXPR
       && TREE_CODE (binrhs) == SSA_NAME
       && acceptable_pow_call (SSA_NAME_DEF_STMT (binrhs), &base, &exponent))
     {
       add_repeat_to_ops_vec (ops, base, exponent);
       gimple_set_visited (SSA_NAME_DEF_STMT (binrhs), true);
     }
   else
     add_to_ops_vec (ops, binrhs);

pattern into a helper that handles the other cases.

Richard.

Richard.

Richard.

Thanks,
Kugan

Reply via email to