On Tue, Apr 18, 2017 at 12:43 PM, Bin Cheng <bin.ch...@arm.com> wrote: > Hi, > This is the major part of this patch series. It rewrites cost computation of > ivopts using tree affine. > Apart from description given by cover message: > A) New computation cost model. Currently, there are big amount code trying > to understand > tree expression and estimate its computation cost. The model is > designed long ago > for generic tree expressions. In order to process generic expression > (even address > expression of array/memory references), it has code for too many corner > cases. The > problem is it's somehow impossible to handle all complicated > expressions, even with > complicated logic in functions like get_computation_cost_at, > difference_cost, > ptr_difference_cost, get_address_cost and so on... The second problem > is it's hard > to keep cost model consistent among special cases. As special cases > being added > from time to time, the model is no long unified any more. There are > cases that right > cost results in bad code, or vice versa, wrong cost results in good > code. Finally, > it's difficult to add code for new cases. > This patch introduces a new cost computation model by using tree affine. > Tree exprs > are lowered to aff_tree which is simple arithmetic operation usually. > Code handling > special cases is no longer necessary, which brings us quite simplicity. > It is also > easier to compute consistent costs among different expressions using > tree affine, > which gives us a unified cost model. > This patch also fixes issue that cost computation for address type iv_use is > inconsistent > with how it is re-rewritten in the end. It greatly simplified cost > computation. > > Is it OK?
The patch is quite hard to follow (diff messes up here -- this is a case where a context diff is easier to read). I trust you on the implementation details here, the overall structure looks ok to me. The only question I have is with regarding to get_loop_invariant_expr which seems to be much less sophisticated than before (basically it's now what was record_inv_expr?). I suppose the old functionality is superseeded by using affines everywhere else. Otherwise ok. Thanks, Richard. > Thanks, > bin > 2017-04-11 Bin Cheng <bin.ch...@arm.com> > > * tree-ssa-loop-ivopts.c (get_loop_invariant_expr): Simplify. > (adjust_setup_cost): New parameter supporting round up adjustment. > (struct address_cost_data): Delete. > (force_expr_to_var_cost): Don't bound cost with spill_cost. > (split_address_cost, ptr_difference_cost): Delete. > (difference_cost, compare_aff_trees, record_inv_expr): Delete. > (struct ainc_cost_data): New struct. > (get_address_cost_ainc): New function. > (get_address_cost, get_computation_cost): Reimplement. > (determine_group_iv_cost_address): Record inv_expr for all uses of > a group. > (determine_group_iv_cost_cond): Call get_loop_invariant_expr. > (iv_ca_has_deps): Reimplemented to ... > (iv_ca_more_deps): ... this. Check if NEW_CP introduces more deps > than OLD_CP. > (iv_ca_extend): Call iv_ca_more_deps.
