On Wed, Apr 26, 2017 at 12:12 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Wed, Apr 26, 2017 at 10:50 AM, Richard Biener > <richard.guent...@gmail.com> wrote: >> 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 > Hi Richard, > Thanks for reviewing, attachment is the updated context diff. It also > includes a minor fix handling pre-increment addressing mode, > specifically, it adds two lines of code: > > if (stmt_after_increment (data->current_loop, cand, use->stmt)) > ainc_offset += ainc_step; > > >> 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. > Yes, previous version tries a lot to cancel common sub expression when > representing use with cand. New version relies on tree affine which > is much better. One problem with invariant expression estimation is > we don't know in IVOPT if the inv_expr would be hoisted or not by > later passes. This problem exists all the time, we can only make > assumptions here, I think new version is bit more aggressive in > recording new inv_expr here.
Thanks. LGTM. Richard. > Thanks, > bin >> >> 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.
