On Fri, May 20, 2016 at 1:51 AM, Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org> wrote: > 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.
I like it. Please push the insertion code to a helper as I think you need to post-pone setting the stmts UID to that point. Ideally we'd make use of the same machinery in attempt_builtin_powi, removing the special-casing of powi_result. (same as I said that ideally the plus->mult stuff would use the repeat-ops machinery...) I'm not 100% convinced the place you insert the stmt is correct but I haven't spent too much time to decipher reassoc in this area. Thanks, Richard. > 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.
