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

Reply via email to