On 10/26/2017 06:06 AM, Richard Biener wrote: > On Mon, Oct 23, 2017 at 1:25 PM, Richard Sandiford > <richard.sandif...@linaro.org> wrote: >> This patch adds a stub helper routine to provide the mode >> of a scalar shift amount, given the mode of the values >> being shifted. >> >> One long-standing problem has been to decide what this mode >> should be for arbitrary rtxes (as opposed to those directly >> tied to a target pattern). Is it the mode of the shifted >> elements? Is it word_mode? Or maybe QImode? Is it whatever >> the corresponding target pattern says? (In which case what >> should the mode be when the target doesn't have a pattern?) >> >> For now the patch picks word_mode, which should be safe on >> all targets but could perhaps become suboptimal if the helper >> routine is used more often than it is in this patch. As it >> stands the patch does not change the generated code. >> >> The patch also adds a helper function that constructs rtxes >> for constant shift amounts, again given the mode of the value >> being shifted. As well as helping with the SVE patches, this >> is one step towards allowing CONST_INTs to have a real mode. > > I think gen_shift_amount_mode is flawed and while encapsulating > constant shift amount RTX generation into a gen_int_shift_amount > looks good to me I'd rather have that ??? in this function (and > I'd use the mode of the RTX shifted, not word_mode...). > > In the end it's up to insn recognizing to convert the op to the > expected mode and for generic RTL it's us that should decide > on the mode -- on GENERIC the shift amount has to be an > integer so why not simply use a mode that is large enough to > make the constant fit? > > Just throwing in some comments here, RTL isn't my primary > expertise. I wonder if encapsulation + a target hook to specify the mode would be better? We'd then have to argue over word_mode, vs QImode vs something else for the default, but at least we'd have a way for the target to specify the mode is generally best when working on shift counts.
In the end I doubt there's a single definition that is overall better. Largely because I suspect there are times when the narrowest mode is best, or the mode of the operand being shifted. So thoughts on doing the encapsulation with a target hook to specify the desired mode? Does that get us what we need for SVE and does it provide us a path forward on this issue if we were to try to move towards CONST_INTs with modes? jeff