On Sat, Aug 05, 2017 at 11:07:20AM +0100, Richard Sandiford wrote:
> Segher Boessenkool <seg...@kernel.crashing.org> writes:
> > This series creates pattern_cost and insn_cost functions that together
> > replace the existing insn_rtx_cost function.
> > pattern_cost is like the old insn_rtx_cost function; insn_cost takes
> > an actual rtx_insn * as input, not just a pattern.
> > Also a targetm.insn_cost is added, which targets can use to implement
> > a more exact cost more easily.
> > The combine patch is pretty gross (but functional), it needs some
> > refactoring (to not call recog so often). The rs6000 patch is very
> > much a work in progress.
> > How does this look? Is this the right direction?
> Seems good to me FWIW. Since this hook is entirely new, would it
> be worth standardising on attribute names for size and speed costs,
> a bit like "length" and "enabled"? I think otherwise the target hooks
> are going to end up with similar boilerplate.
For size cost I currently use just "length", but I haven't looked at
size cost much at all yet.
For speed cost I primarily use "type", modified by the number of machine
insns a pattern generates (quite a few are split); and I get the number
of machine insns just from "length" again, which for rs6000 is easy and
correct in most cases. Some other targets may need something else.
I also have an attribute "cost" that can be used to override the
default calculation; that seems useful to standardise on. I've pondered
a "cost_adjust" that will be added to the calculated cost instead, but
it hasn't been useful so far.
There are two types of insns that happen in the insn stream but which
aren't recognised: asm, and no-op moves. I cost both as cost 0 currently.
"length" and "enabled" are special, they are used by non-target code.
So far I don't think it is useful to handle "cost" specially, but it
is such a nice short name that I expect many targets will want it,
whether it is handled generically or not :-)