Re: [PATCH GCC8][13/33]Rewrite cost computation of ivopts

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Apr 18, 2017 at 12:43 PM, Bin Cheng <Bin.Cheng@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.cheng@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.