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. Jeff