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

["Bin.Cheng" <amker dot cheng at gmail dot com>]

On Wed, Apr 26, 2017 at 10:50 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> 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
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,
bin
>
> 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.

Attachment: 0013-rewrite-cost-computation-20170401.txt
Description: Text document