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 11:18 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Apr 26, 2017 at 12:12 PM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> 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.
>
> LGTM.
Trivial update replacing calls to aff_combination_simple_p because of
change in previous patch.

Thanks,
bin
>
> Richard.
>
>> 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-20170227.txt
Description: Text document