> From: Richard Sandiford <rdsandif...@googlemail.com> > Date: Wed, 9 May 2012 11:14:49 +0200
> Hans-Peter Nilsson <hans-peter.nils...@axis.com> writes: > >> From: Richard Sandiford <rdsandif...@googlemail.com> > >> Date: Tue, 1 May 2012 16:46:38 +0200 > > > >> To repeat: as things stand, very few targets define proper rtx costs > >> for SET. > > > > IMHO it's wrong to start blaming targets when rtx_cost doesn't > > take the mode in account in the first place, for the default > > cost. (Well, except for the modes-tieable subreg special-case.) > > The targets where an operation in N * word_mode costs no more > > than one in word_mode, if there even is one, is a minority, > > let's adjust the defaults to that. > > I'll pass on approving or disapproving this patch, but for the record: > a factor of word_mode seems a bit too simplistic. I'd say it's the right level: simplistic enough for the default, not to mention now linear, without being plainly ignorant as now. > It's OK for moves > and logic ops, but addition of multiword modes is a bit more complicated. How about (same factor) factor*COSTS_N_INSNS (1)*3/2 to account for carry? Or is 2*factor a better default? Further improvements are welcome, but I see the patch as a strict improvement and I hope it will not be shot down by requests for perfection - at least not without detailing said perfection. > Multiplication and division by multiword modes is even more > so, of course. Suggestions are welcome, but in the absence of that, I'd say any factor larger than one is is a good start, like in the patch. > > I think there should be a gating check whether the target > > implements that kind of shift in that mode at all, before > > checking the cost. Not sure whether it's generally best to put > > that test here, or to make the rtx_cost function return the cost > > of a libcall for that mode when that happens. Similar for the > > other insns. > > This has come up a few times in past discussions about rtx_cost > (as I'm sure you remember :-)). On the one hand, I agree it might > be nice to shield the backend from invalid insns. That would > effectively mean calling expand on each insn though, which would be > rather expensive. No, nothing that complicated. I'm thinking of just basically checking that there's an operation in that mode, like: if (direct_optab_handler (code_to_optab [GET_CODE (x)], mode) == CODE_FOR_nothing) { ... return tabled default cost; for libcall or open-code ... } Restricting the validity-gating to checking that the mode is valid for the operation wouldn't interfere with fancy pipeline speculative use. > So I think this patch is using rtx_cost according to its current > interface. The "interface" use previously ignored the mode for most uses (QED), so that's not completely correct. ;) > If someone wants to change or restrict that interface, > than that's a separate change IMO. And it should be done consistently > rather than in this one place. > > In this case it doesn't matter anyway. If we never see a shift > in mode M by amount X, we'll never need to make a decision about > whether to split it. If it's never used, then I don't mind it being wrong if that simplifies the computation. :) > > Isn't the below better than doing virtually the same in each > > target's rtx_costs? > > FWIW, MIPS, SH and x86 all used something slightly different (and more > complicated). I imagine PowerPC and SPARC will too. So "each" seems > a bit strong. > > That's not an objection to the patch though. I realise some ports do > have very regular architectures where every register is the same width > and has the same move cost. Hence the default should follow a very regular model... brgds, H-P