On 08/05/2017 11:15 AM, Segher Boessenkool wrote:
> 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.
I think that's fine. "length" is pretty standardized at this point and
it's the right metric. For ports that don't bother defining a length
attribute, punt in some reasonable manner.
> 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.
Let's go ahead and "reserve" cost and cost_adjust for this purpose. If
we find that cost_adjust isn't actually necessary, then so be it, it's
not a big deal to me.