This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, tree-optimization] Fix for PR 52868


On Wed, Apr 25, 2012 at 4:32 PM, Igor Zamyatin <izamyatin@gmail.com> wrote:
> On Wed, Apr 25, 2012 at 1:14 PM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Apr 25, 2012 at 10:56 AM, Igor Zamyatin <izamyatin@gmail.com> wrote:
>>> Hi all!
>>>
>>> I'd like to post for review the patch which makes some costs adjusting
>>> in get_computation_cost_at routine in ivopts part.
>>> As mentioned in the PR changes also fix the bwaves regression from PR 52272.
>>> Also changes introduce no degradations on spec2000/2006 and
>>> EEMBC1.1/2.0(this was measured on Atom) on x86
>>>
>>>
>>> Bootstrapped and regtested on x86. Ok to commit?
>>
>> I can't make sense of the patch and the comment does not help.
>>
>> + ? ? ?diff_cost = cost.cost;
>> ? ? ? cost.cost /= avg_loop_niter (data->current_loop);
>> + ? ? ?add_cost_val = add_cost (TYPE_MODE (ctype), data->speed);
>> + ? ? ?/* Do cost correction if address cost is small enough
>> + ? ? ? ? and difference cost is high enough. ?*/
>> + ? ? ?if (address_p && diff_cost > add_cost_val
>> + ? ? ? ? ?&& get_address_cost (symbol_present, var_present,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? offset, ratio, cstepi,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TYPE_MODE (TREE_TYPE (utype)),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TYPE_ADDR_SPACE (TREE_TYPE (utype)),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? speed, stmt_is_after_inc,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? can_autoinc).cost <= add_cost_val)
>> + ? ? ? ?cost.cost += add_cost_val;
>>
>> Please explain more thoroughly. ?It also would seem to be better to add
>> an extra case, as later code does
>
> For example for such code
>
> ? for (j=0; j<M;j++) {
> ? ? ? for (i=0; i<N; i++)
> ? ? ? ? ? sum += ptr->a[j][i] * ptr->c[k][i];
> ? }
> ?we currently have following gimple on x86 target (I provided a piece
> of all phase output):
>
> ? ? ? ? ? # ivtmp.13_30 = PHI <ivtmp.13_31(3), ivtmp.13_33(7)>
> ? ? ? ? ? D.1748_34 = (void *) ivtmp.13_30;
> ? ? ? ? ? D.1722_7 = MEM[base: D.1748_34, offset: 0B];
> ? ? ? ? ? D.1750_36 = ivtmp.27_28;
> ? ? ? ? ? D.1751_37 = D.1750_36 + ivtmp.13_30; <-- we got
> non-invariant add which is not taken into account currently in cost
> model
> ? ? ? ? ? D.1752_38 = (void *) D.1751_37;
> ? ? ? ? ? D.1753_39 = (sizetype) k_8(D);
> ? ? ? ? ? D.1754_40 = D.1753_39 * 800;
> ? ? ? ? ? D.1723_9 = MEM[base: D.1752_38, index: D.1754_40, offset: 16000B];
> ? ? ? ? ? ...
>
> ?With proposed fix we produce:
>
> ? ? ? ? ? # ivtmp.14_30 = PHI <ivtmp.14_31(3), 0(7)>
> ? ? ? ? ? D.1749_34 = (struct S *) ivtmp.25_28;
> ? ? ? ? ? D.1722_7 = MEM[base: D.1749_34, index: ivtmp.14_30, offset: 0B];
> ? ? ? ? ? D.1750_35 = (sizetype) k_8(D);
> ? ? ? ? ? D.1751_36 = D.1750_35 * 800;
> ? ? ? ? ? D.1752_37 = ptr_6(D) + D.1751_36;
> ? ? ? ? ? D.1723_9 = MEM[base: D.1752_37, index: ivtmp.14_30, offset: 16000B];
>
> which is more effective on platforms where address cost is cheaper
> than cost of addition operation. That's basically what this adjustment
> is for.

If we generally miss to account for the add then why is the adjustment
conditional on diff_cost > add_cost and address_cost <= add_cost?

Is this a new heuristic or a fix for not accurately computing the cost for the
stmts we generate?

Richard.

> So comment in the source code now looks as follows
>
> /* Do cost correction when address difference produces
> ? additional non-invariant add operation which is less
> ? profitable if address cost is cheaper than cost of add. ?*/
>
>>
>> ?/* Now the computation is in shape symbol + var1 + const + ratio * var2.
>> ? ? (symbol/var1/const parts may be omitted). ?If we are looking for an
>> ? ? address, find the cost of addressing this. ?*/
>> ?if (address_p)
>> ? ?return add_costs (cost,
>> ? ? ? ? ? ? ? ? ? ? ?get_address_cost (symbol_present, var_present,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?offset, ratio, cstepi,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_MODE (TREE_TYPE (utype)),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_ADDR_SPACE (TREE_TYPE (utype)),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?speed, stmt_is_after_inc,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?can_autoinc));
>>
>> thus refactoring the code a bit would make it possible to CSE the
>> get_address_cost
>> call and eventually make it clearer what the code does.
>
> 'offset' could be changed beetween two calls of get_address_cost so
> such refactoring looks useless.
>
> New patch (only the comment was changed) attached. Changelog was
> changed as well.
>
>>
>> Richard.
>>
>
> Changelog:
>
> ?2012-04-26 ?Yuri Rumyantsev ?<yuri.rumyantsev@intel.com>
>
> ?? ? ? ?* tree-ssa-loop-ivopts.c (get_computation_cost_at): Adjust
> ?? ? ? cost model?when address difference produces additional
> ? ? ? ?non-invariant add operation which is less profitable if
> ? ? ? ?address cost is cheaper than cost of add.
>
> Thanks,
> Igor


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]