Hi Bin,
On 4 May 2017 at 17:25, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Wed, Apr 26, 2017 at 11:18 AM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> 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.
> Trivial update replacing calls to aff_combination_simple_p because of
> change in previous patch.
>
> Thanks,
> bin
This commit (r247885) causes regressions on aarch64:
- PASS now FAIL [PASS => FAIL]:
Executed from: gcc.target/aarch64/aarch64.exp
gcc.target/aarch64/pr62178.c scan-assembler ld1r\\t{v[0-9]+.
Christophe
>>
>> 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.
