On Thu, 25 Oct 2012, Jan Hubicka wrote:
> > > +/* For operands of load/stores estimate cost of the address computations
> > > + involved. */
> > > +
> > > +static int
> > > +estimate_operand_cost (tree op)
> > > +{
> > > + int cost = 0;
> > > + while (handled_component_p (op))
> > > + {
> > > + cost += estimate_ref_cost (op);
> > > + op = TREE_OPERAND (op, 0);
> > > + }
> > > + /* account (TARGET_)MEM_REF. */
> > > + return cost + estimate_ref_cost (op);
> >
> > ICK ...
> >
> > Why not sth as simple as
> >
> > return num_ssa_operands (stmt, SSA_OP_USE);
> >
> > ? a[1][2] and b[2] really have the same cost, variable length
> > objects have extra SSA operands in ARRAY_REF/COMPONENT_REF for
> > the size. Thus, stmt cost somehow should reflect the number
> > of dependent stmts.
> >
> > So in estimate_num_insns I'd try
> >
> > int
> > estimate_num_insns (gimple stmt, eni_weights *weights)
> > {
> > unsigned cost, i;
> > enum gimple_code code = gimple_code (stmt);
> > tree lhs;
> > tree rhs;
> >
> > switch (code)
> > {
> > case GIMPLE_ASSIGN:
> > /* Initialize with the number of SSA uses, one is free. */
> > cost = num_ssa_operands (stmt, SSA_OP_USE);
> > if (cost > 1)
> > --cost;
> >
> > ...
>
> Hmm, also in ASM/call/return?
We have lots of special casing in the call / return / asm case already,
so to ...
> It will definitely make quite a fuzz into the cost metric
... not do this too much I'd even restrict it to memory loads / stores
(for your address cost). Maybe also for calls (they can have memory
loads as arguments and store location), or asms.
> by making a=b+c to have cost of 3 instead of 1 as it have now. I am not 100%
> sure if
> a+b should be more expensive than a+1.
Yeah, that's why I made it num_ssa_operands - 1 ;) Also because
of SSA name = SSA name considered to having cost 0.
> I can give that a try and we will see after re-tunning how well it works.
> Diagonal walk
> of multi-dimensional array, i.e. a[b][b][b] will be mis-accounted, too, right?
Well, it has 3 SSA operands but also three multiplications.
I'd say try it for loads / stores in GIMPLE_ASSIGNs only for now.
Or slightly advanced, count the number of SSA names in each memory
operand (you can't use num_ssa_operands then but have to resort to
tree walks like your patch does, just less "special"-casing)
Richard.
