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 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.

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

Attachment: ivopts1.patch
Description: Binary data


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