On Wed, Jul 17, 2019 at 1:32 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On Wed, Jul 17, 2019 at 12:54 PM Jakub Jelinek <ja...@redhat.com> wrote: > > > > On Wed, Jul 17, 2019 at 12:37:59PM +0200, Richard Biener wrote: > > > On Tue, Jul 16, 2019 at 10:45 AM Kewen.Lin <li...@linux.ibm.com> wrote: > > > > Based on the previous comments (thank you!), I tried to update the > > > > handling in expander and vectorizer. Middle-end optimizes lrotate > > > > with const rotation count to rrotate all the time, it makes vectorizer > > > > fail to vectorize if rrotate isn't supported on the target. We can at > > > > least teach it on const rotation count, the cost should be the same? > > > > At the same time, the expander already tries to use the opposite > > > > rotation optable for scalar, we can teach it to deal with vector as > > > > well. > > > > > > > > Is it on the right track and reasonable? > > > > > > So you're basically fixing this up in the expander. I think on > > > the GIMPLE level you then miss to update tree-vect-generic.c? > > > > > > I'm not sure if it makes sense to have both LROTATE_EXPR and > > > RROTATE_EXPR on the GIMPLE level then (that CPUs only > > > support one direction is natural though). So maybe simply get > > > rid of one? Its semantics are also nowhere documented > > > > A lot of targets support both, and I think not all targets do the > > truncation, so at least with non-constant rotate count emitting one over the > > other is important and trying to match it up only during combine might be > > too late and not work well in many cases. > > Then there are some targets that only support left rotates and not right > > rotates (rs6000, s390, tilegx, ...), and other targets that only support > > right rotates (mips, iq2000, ...). > > So only having one GIMPLE code doesn't seem to be good enough. > > It seems for constants it is by means of canonicalization. The lack > of canonicalization for non-constants then makes us fail to CSE > lrotate<x, a> and rrotate<x, width-a>. Given rotates are only > detected on GIMPLE always creating one or the other should be > reasonably easy and fixup during expansion can happen either via TER > or via pre-expand generation of optab corresponding IFNs? > > Might get tricky if we face width - (a + 5) so the pattern matching > of an opposing direction rotate gets harder. > > > I think handling it during expansion in generic code is fine, especially > > when we clearly have several targets that do support only one of the > > rotates. > > Yes. > > > As you wrote, it needs corresponding code in tree-vect-generic.c, > > and shouldn't hardcode the rs6000 direction of mapping rotr to rotl, but > > support also the other direction - rotl to rotr. For the sake of > > !SHIFT_COUNT_TRUNCATED targets for constant shift counts it needs to do > > negation + masking and for variable shift counts probably punt and let the > > backend code handle it if it can do the truncation in there? > > Ick. I wouldn't even touch SHIFT_COUNT_TRUNCATED with a 10-foot pole > here. And for rotates we can simply always truncate constant amounts to > the rotated operands width, no? For non-constant ones I fear targets > would need to support both to get reliable expansion.
Btw, the docs of SHIFT_COUNT_TRUNCATED do not mention rotates unless you treat a rotate as a shift. Richard. > Richard. > > > Jakub
