[PATCH] D157417: [RISCV][SelectionDAG] Lower shuffles as bitrotates with vror.vi when possible

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

This might be an over generalization, but I can't help but notice that we don't need the same rotate amount in each chunk.  Given the small number of possible rotate amounts, I wonder if we'd be worth generating a rotate by a build_vector.  This might be overly general.

When looking at your tests, I noticed a bunch which should probably be vrev8.v instead.  Might be worth implementing that, then rebasing this over.  I'd be fine with the other order too, so don't take that as must.

Broader point is that maybe we should be doing this even without zbb.



================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-rotate.ll:191
+
+define <8 x i8> @shuffle_v8i8_as_i16(<8 x i8> %v) {
+; CHECK-LABEL: shuffle_v8i8_as_i16:
----------------
This case is interesting a couple different ways.

First, the vror here is also a vrev8.v.  This points out that we probably should be matching brev8.v.  Minor, but we should probably also be preferring it here when both are possible.

Second, the bswap lowering for 2 element pairs is going to be cheaper than the vrgather even without zvbb.  We probably should be matching high/low swapping on all element sizes up to XLEN.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-rotate.ll:239
+; ZVBB_ZVE32X-NEXT:    ret
+  %shuffle = shufflevector <8 x i8> %v, <8 x i8> poison, <8 x i32> <i32 1, i32 2, i32 3, i32 0, i32 5, i32 6, i32 7, i32 4>
+  ret <8 x i8> %shuffle
----------------
Continuing from the prior comment..

Since this isn't an arbitrary byte shuffle within each element, but instead two contiguous regions, I think using the rotate expansion even for no-zbb is going to be profitable.  


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157417/new/

https://reviews.llvm.org/D157417