Hello Richard,
Thanks a lot for the review!
> it's not easy to follow the flow of this function, esp. I wonder
>
> + else
> + {
> + tree var = create_tmp_reg (TREE_TYPE (last_rhs1), "reassoc");
> + add_referenced_var (var);
> + stmts[i] = build_and_add_sum (var, op1, op2, opcode);
> + }
>
> what you need to create new stmts for, and if you possibly create
> multiple temporary variables here.
>
> - TODO_verify_ssa
> + TODO_remove_unused_locals
> + | TODO_verify_ssa
>
> why?
>
Current rewrite_expr_tree uses original statements and just change
order of operands (tree leafs) usage. To keep data flow correct it
moves all statements down to the last one. I tried such approach but
it did not work well because of increased registers pressure in some
cases (all operands are live at the same time right before the first
statement of our sequence). Then I tried to move existing statements
to the appropriate places (just after the last operand definition) but
it appeared to be not so easy. It is easy to guarantee consistence in
the final result but not easy to keep data flow consistent after each
statement move. And if we do not keep code consistent after each move
then we may get wrong debug statement generated. After few tries I
decided that the most easy for both understanding and implementation
way is to recreate statements and remove the old chain. So, I used
build_and_add_sum to create all statements except the last one which
never needs recreation. I added TODO_remove_unused_locals since temps
used in original statements should be removed. Is this approach OK?
> You don't seem to handle the special-cases of rewrite_expr_tree
> for the leafs of your tree, especially the PHI node special-casing.
> I think you may run into vectorization issues here.
>
I do not see any reason why these cases are checked in
rewrite_expr_tree. It is optimization of operands order and may be it
will be good to move it somewhere. Is it OK if I move related code to
the end of optimize_ops_list method?
Thanks
Ilya
