Jakub Jelinek <[email protected]> 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
