This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] RFC: Hook for insn costs?


Him

On Mon, Jul 17, 2017 at 03:36:13PM +0200, Georg-Johann Lay wrote:
> >On Wed, Jul 12, 2017 at 03:15:09PM +0200, Georg-Johann Lay wrote:
> >>the current cost computations in rtlanal.c and maybe other places
> >>suffer from the fact that they are hiding parts of the expressions
> >>from the back-end, like SET_DESTs of single_set or the anatomy of
> >>PARALELLs.
> >>
> >>Would it be in order to have a hook like the one attached?
> >>
> >>I am aware of that, in an ideal world, there wouldn't be more
> >>than one hook to get rtx costs.  But well...
> >
> >The number of hooks is not a problem.  The overall complexity of this
> >interface (between the backends and optimisers; a group of related
> >hooks) _is_ a problem.  Also, the interface should be good for all
> >targets, easy to use, do one thing and do it well.
> 
> The interface is easy and straight forward, but the new hook might not
> be as convenient as rtx_costs.  For example, testing for single_set is
> a bit tedious when you don't have an insn.

But you probably shouldn't anyway -- you do not care what the only
used set would cost as a stand-alone insn (_if_ it is valid, even!) --
you want to know the cost of the full insn.

> There are situations where rtx_costs is called with outer_code = INSN;
> cf. set_rtx_cost.
> 
> Using set_src_cost instead of a new hook that gracefully degrades
> might make a hell break loose when rtx_costs just return some penalty
> costs for unknown stuff instead of "0".
> 
> Therefore, a new hook is easier to use and understand, but it might
> be confusing in what situations rtx_costs with outer_code = INSN is
> used and in what situations patter_costs would be used.

It won't be confusing if we do a full conversion.  That will also show
if there is anything we missed.

Not very many things use insn_rtx_cost.

> >So I argue that we want to have a hook for insn cost.
> >
> >Now what should it take as input?  An rtx_insn, or just the pattern
> >(as insn_rtx_cost does)?  And if an rtx_insn, should that than be an
> >rtx_insn that is already linked into the insn chain (so we can see what
> >other insns generate its inputs, and which of its outputs are unused,
> >etc.)?
> 
> IMO, if we have an insn then we could also pass it, it doesn't cost
> anything.  However I don't know if the insn might have some strange
> properties like an INSN_CODE that doesn't reflect the actual pattern.
> Ans if reg_notes might be helpful and correct resp. up to date.

And we do not always have an insn.  If we do, we should require all
info there is to be correct, but we likely cannot require all of it.

> It should be in order to run the hook from within a sequence, hence
> "insn chain" might not be what the user expects, i.e. prev_insn might
> be NULL even though the current function already has some insns.

Yes, "an insn not linked into the insn chain".

> >Why not just -1?  And is 0 really so terrible; in the extremely rare
> >case we really want cost 0, it won't hurt much saying "unknown", as we
> >do currently.  For "hook unimplemented", just set the hook to NULL.
> 
> Using "0" is really bad design.  It's not clear when you are reading the
> sources that it is some magic value.  Some identifier to refer to such
> a value is much better.

We'll have to disagree then.

> And 0 is also bad because there might be
> situations where 0 is a legal cost.  There might even be situations
> where you want negative cost in order to prefer a specific pattern
> re. some alternative (cost functions are not always 100% correct and
> are expected costs as they run before reg alloc etc.)

That is going to be fun for those places that use unsigneds to hold
the costs ;-)


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]