Re: [PATCH 0/5] RFC, WIP: RTL cost improvements

[Richard Sandiford <richard dot sandiford at linaro dot org>]

Jeff Law <law@redhat.com> writes:
> 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 <segher@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.

I was thinking we should have separate attributes for size and speed
from the outset.   How about size_cost and speed_cost?  It'd be up
to the target to decide whether to define them as the sums of various
sub-attributes (like it's the target's decision how to break "enabled"
down).

The advantage of doing it all in attributes is that the generator might
be able to help optimise the process of checking for costs.  No promises
though :-)

TBH I think we should start with the attributes as well-defined names
and only add the hook if we find we still need it.  I can't really see
what a C function would give over keeping the costs with the instruction
definitions.

Thanks,
Richard