Re: [PATCH 3/3] Add targetm.insn_cost hook

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Wed, Oct 11, 2017 at 05:39:25PM -0600, Sandra Loosemore wrote:
> On 10/09/2017 01:35 PM, Segher Boessenkool wrote:
> >This adds a new hook that the insn_cost function uses if a target has
> >implemented it (it uses the old pattern_cost nee insn_rtx_cost if not).

> As a target maintainer, I'm kind of confused by this patch, and I don't 
> think the tm.texi change gives sufficient guidance about the default 
> hook behavior, how it interacts with TARGET_RTX_COSTS and/or 
> TARGET_ADDRESS_COST, or the different contexts the three hooks are used 
> in.  Do target maintainers need to do something to define this new hook 
> to prevent performance regressions?

No, everything works as it did if you don't do anything.  Should I add
a little bit of documentation what the hook does by default?  It may be
good as a scary story, to get more ports to use their own hook
implementation instead ;-)

> I could try to write up some advice about cost models and tuning for the 
> internals manual, but at present I don't feel like I have any 
> understanding of what motivated this change or how it changed the 
> recommended practices for back end tuning.  :-(

insn_rtx_cost did not work with instructions: it worked with instruction
_patterns_.  It also does not work for anything but single-set and similar
patterns (not the same as single_set, that requires an instruction!)

It also is really hard to write a TARGET_RTX_COSTS that properly describes
the cost of the machine instructions you have (and to keep it updated!)

With these patches combine can make better decisions already (no longer
do many instructions have "unknown" cost).  I am planning to move more
users of TARGET_RTX_COSTS over to insn_cost, or even get the needed info
in a more direct way.  This will take time; all help is welcome :-)


Segher