Jakub Jelinek <ja...@redhat.com> writes: > On Mon, Jul 15, 2019 at 04:50:13PM +0800, Kewen.Lin wrote: >> In match.pd and expmed.c, we have some codes to transform lrotate to >> rrotate if rotation count is const. But they don't consider the target >> whether supports the rrotate. It leads to some suboptimal generated >> code since some optimization can't get expected result by querying >> target optab. One typical case is that we miss some rotation >> vectorization opportunity on Power. > > This will not do the right thing if neither lrotate nor rrotate is > supported, you want to canonicalize in that case IMHO. > The code formatting is wrong (|| at the end of line, overly long lines). > > Finally, what is the reason why Power doesn't support one of the rotates? > Especially for rotates by constant, you can transform those in the > define_expand. > > Canonicalizing code is highly desirable during GIMPLE, it means if you have > say left rotate by 23 in one spot and right rotate by bitsize - 23 in > another spot, e.g. SCCVN can merge that code. > > So, IMNSHO, just improve the backend.
Agreed FWIW, if the target supports rotates by a constant. If it only supports rotates by a variable then I think expand should try to use the rrotate optab for lrotates. The current code looks odd though. The comment above the expmed.c hunk is: /* Canonicalize rotates by constant amount. If op1 is bitsize / 2, prefer left rotation, if op1 is from bitsize / 2 + 1 to bitsize - 1, use other direction of rotate with 1 .. bitsize / 2 - 1 amount instead. */ whereas rtl.texi says: @item (rotate:@var{m} @var{x} @var{c}) @itemx (rotatert:@var{m} @var{x} @var{c}) Similar but represent left and right rotate. If @var{c} is a constant, use @code{rotate}. simplify-rtx.c seems to stick to the rtl.texi version and use ROTATE rather than ROTATERT for constant rotates. If a target prefers the expmed.c version then I think it should handle that in the asm template. Thanks, Richard