[PATCH] D13851: [X86][XOP] Add support for lowering vector rotations

[Simon Pilgrim via llvm-commits]

RKSimon added a comment.

In http://reviews.llvm.org/D13851#271893, @delena wrote:

> In http://reviews.llvm.org/D13851#271535, @RKSimon wrote:
>
> > Updated with 256-bit tests. I've removed ROTR lowering from XOP for now - we don't have any combines that just create ROTR (all attempt to lower with either ROTL/ROTR depending on what is legal/custom), so we can avoid the extra cost of the subtract and always use ROTL on XOP.
>
>
> The rotation immediate can be negative. It will mean rotation right. I assume that the combiner catches this scenario and generates ROTL with negative value. Could you, please, add a test for this?


I've tried to create such a test but without success - similar to shifts, for the ISD rotate ops any value less than 0 or greater than/equal to bitsize is undefined so I consider this to be correct for the XOP lowering. I've already removed the unnecessary XOP lowering of ISD::ROTR.

Would updating the lowering assert to check that the (sign extended rotation) constants are in range be satisfactory instead? Failing that I would have to add a ((1 << bitsize) - 1) mask to force the rotation to be in range which I believe isn't necessary in the same way that we don't do this for AVX2/XOP shifts by variable.


Repository:
  rL LLVM

http://reviews.llvm.org/D13851