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. 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