[RISCV] Canonicalize vrot{l,r} to vrev8 when lowering shuffle as rotate

[luke]

Summary

A rotate of 8 bits of an e16 vector in either direction is equivalent to a
byteswap, i.e. vrev8. There is a generic combine on ISD::ROT{L,R} to
canonicalize these rotations to byteswaps, but on fixed vectors they are
legalized before they have the chance to be combined. This patch teaches the
rotate vector_shuffle lowering to emit these rotations as byteswaps to match
the scalable vector behaviour.

[craig.topper]

Do we have reason to believe vrev8 is better than vror?

[luke]

In D158195#4595901, @craig.topper wrote:

Do we have reason to believe vrev8 is better than vror?

@reames alluded to it here: https://reviews.llvm.org/D157417#inline-1529308
Tangentially, DAGCombiner canonicalises rotr/rotl to bswap anyway so this brings fixed-length vector behaviour more in line with scalable

[craig.topper]

If the rotate came in as a fshl/fshr intrinsic or as shl+shr+or would we already get vrev8 for fixed vectors? Is only the shuffle case that is being optimized?

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse.ll

278

How does this patch create new rotates?

[luke]

In D158195#4596052, @craig.topper wrote:

If the rotate came in as a fshl/fshr intrinsic or as shl+shr+or would we already get vrev8 for fixed vectors? Is only the shuffle case that is being optimized?

Yeah we already get vrev8 for these, DAGCombiner canonicalises them before they would be legalised to vl nodes:

define <4 x i16> @rot_via_fshr(<4 x i16> %a) {
  %res = call <4 x i16> @llvm.fshr.v4i16(<4 x i16> %a, <4 x i16> %a, <4 x i16> <i16 8, i16 8, i16 8, i16 8>)
  ret <4 x i16> %res
}

declare <4 x i16> @llvm.fshr.v4i16(<4 x i16> %a, <4 x i16> %b, <4 x i16> %c)

define <4 x i16> @rot_via_shift(<4 x i16> %a, <4 x i16> %amt) {
  %1 = shl <4 x i16> %a, <i16 8, i16 8, i16 8, i16 8>
  %2 = lshr <4 x i16> %a, <i16 8, i16 8, i16 8, i16 8>
  %3 = or <4 x i16> %1, %2
  ret <4 x i16> %3
}

=== rot_via_fshr
Initial selection DAG: %bb.0 'rot_via_fshr:'
SelectionDAG has 13 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
        t6: v4i16 = BUILD_VECTOR Constant:i16<8>, Constant:i16<8>, Constant:i16<8>, Constant:i16<8>
      t7: v4i16 = rotr t4, t6
    t9: nxv2i16 = insert_subvector undef:nxv2i16, t7, Constant:i64<0>
  t11: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t9
  t12: ch = RISCVISD::RET_GLUE t11, Register:nxv2i16 $v8, t11:1


Optimized lowered selection DAG: %bb.0 'rot_via_fshr:'
SelectionDAG has 11 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
      t13: v4i16 = bswap t4
    t9: nxv2i16 = insert_subvector undef:nxv2i16, t13, Constant:i64<0>
  t11: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t9
  t12: ch = RISCVISD::RET_GLUE t11, Register:nxv2i16 $v8, t11:1

=== rot_via_shift
Initial selection DAG: %bb.0 'rot_via_shift:'
SelectionDAG has 18 nodes:
  t0: ch,glue = EntryToken
    t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
  t4: v4i16 = extract_subvector t2, Constant:i64<0>
    t6: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %1
  t7: v4i16 = extract_subvector t6, Constant:i64<0>
  t9: v4i16 = BUILD_VECTOR Constant:i16<8>, Constant:i16<8>, Constant:i16<8>, Constant:i16<8>
        t10: v4i16 = shl t4, t9
        t11: v4i16 = srl t4, t9
      t12: v4i16 = or t10, t11
    t14: nxv2i16 = insert_subvector undef:nxv2i16, t12, Constant:i64<0>
  t16: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t14
  t17: ch = RISCVISD::RET_GLUE t16, Register:nxv2i16 $v8, t16:1


Optimized lowered selection DAG: %bb.0 'rot_via_shift:'
SelectionDAG has 11 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
      t19: v4i16 = bswap t4
    t14: nxv2i16 = insert_subvector undef:nxv2i16, t19, Constant:i64<0>
  t16: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t14
  t17: ch = RISCVISD::RET_GLUE t16, Register:nxv2i16 $v8, t16:1

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse.ll

278

Not sure how I didn't notice these. Looks like it always emitted rotates on zvbb, there's just an issue with the filecheck prefixes.

[luke]

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse.ll

278

*It emits rotates on zvbb after D157417

[luke]

Fix extraneous test diffs caused by dodgy rebase (filecheck prefixes are actually fine:
sorry for the noise)

[craig.topper]

In D158195#4598747, @luke wrote:

In D158195#4596052, @craig.topper wrote:

If the rotate came in as a fshl/fshr intrinsic or as shl+shr+or would we already get vrev8 for fixed vectors? Is only the shuffle case that is being optimized?

Yeah we already get vrev8 for these, DAGCombiner canonicalises them before they would be legalised to vl nodes:

define <4 x i16> @rot_via_fshr(<4 x i16> %a) {
  %res = call <4 x i16> @llvm.fshr.v4i16(<4 x i16> %a, <4 x i16> %a, <4 x i16> <i16 8, i16 8, i16 8, i16 8>)
  ret <4 x i16> %res
}

declare <4 x i16> @llvm.fshr.v4i16(<4 x i16> %a, <4 x i16> %b, <4 x i16> %c)

define <4 x i16> @rot_via_shift(<4 x i16> %a, <4 x i16> %amt) {
  %1 = shl <4 x i16> %a, <i16 8, i16 8, i16 8, i16 8>
  %2 = lshr <4 x i16> %a, <i16 8, i16 8, i16 8, i16 8>
  %3 = or <4 x i16> %1, %2
  ret <4 x i16> %3
}

=== rot_via_fshr
Initial selection DAG: %bb.0 'rot_via_fshr:'
SelectionDAG has 13 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
        t6: v4i16 = BUILD_VECTOR Constant:i16<8>, Constant:i16<8>, Constant:i16<8>, Constant:i16<8>
      t7: v4i16 = rotr t4, t6
    t9: nxv2i16 = insert_subvector undef:nxv2i16, t7, Constant:i64<0>
  t11: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t9
  t12: ch = RISCVISD::RET_GLUE t11, Register:nxv2i16 $v8, t11:1


Optimized lowered selection DAG: %bb.0 'rot_via_fshr:'
SelectionDAG has 11 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
      t13: v4i16 = bswap t4
    t9: nxv2i16 = insert_subvector undef:nxv2i16, t13, Constant:i64<0>
  t11: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t9
  t12: ch = RISCVISD::RET_GLUE t11, Register:nxv2i16 $v8, t11:1

=== rot_via_shift
Initial selection DAG: %bb.0 'rot_via_shift:'
SelectionDAG has 18 nodes:
  t0: ch,glue = EntryToken
    t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
  t4: v4i16 = extract_subvector t2, Constant:i64<0>
    t6: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %1
  t7: v4i16 = extract_subvector t6, Constant:i64<0>
  t9: v4i16 = BUILD_VECTOR Constant:i16<8>, Constant:i16<8>, Constant:i16<8>, Constant:i16<8>
        t10: v4i16 = shl t4, t9
        t11: v4i16 = srl t4, t9
      t12: v4i16 = or t10, t11
    t14: nxv2i16 = insert_subvector undef:nxv2i16, t12, Constant:i64<0>
  t16: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t14
  t17: ch = RISCVISD::RET_GLUE t16, Register:nxv2i16 $v8, t16:1


Optimized lowered selection DAG: %bb.0 'rot_via_shift:'
SelectionDAG has 11 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
      t19: v4i16 = bswap t4
    t14: nxv2i16 = insert_subvector undef:nxv2i16, t19, Constant:i64<0>
  t16: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t14
  t17: ch = RISCVISD::RET_GLUE t16, Register:nxv2i16 $v8, t16:1

How ugly would it be to do it as a special case during the shuffle lowering instead?

[reames]

In D158195#4595909, @luke wrote:

In D158195#4595901, @craig.topper wrote:

Do we have reason to believe vrev8 is better than vror?

@reames alluded to it here: https://reviews.llvm.org/D157417#inline-1529308
Tangentially, DAGCombiner canonicalises rotr/rotl to bswap anyway so this brings fixed-length vector behaviour more in line with scalable

For context, I do *not* think there's a performance difference. This was mostly a canonicalization thing.

[reames]

In D158195#4599594, @craig.topper wrote:

In D158195#4598747, @luke wrote:

In D158195#4596052, @craig.topper wrote:

If the rotate came in as a fshl/fshr intrinsic or as shl+shr+or would we already get vrev8 for fixed vectors? Is only the shuffle case that is being optimized?

Yeah we already get vrev8 for these, DAGCombiner canonicalises them before they would be legalised to vl nodes:

define <4 x i16> @rot_via_fshr(<4 x i16> %a) {
  %res = call <4 x i16> @llvm.fshr.v4i16(<4 x i16> %a, <4 x i16> %a, <4 x i16> <i16 8, i16 8, i16 8, i16 8>)
  ret <4 x i16> %res
}

declare <4 x i16> @llvm.fshr.v4i16(<4 x i16> %a, <4 x i16> %b, <4 x i16> %c)

define <4 x i16> @rot_via_shift(<4 x i16> %a, <4 x i16> %amt) {
  %1 = shl <4 x i16> %a, <i16 8, i16 8, i16 8, i16 8>
  %2 = lshr <4 x i16> %a, <i16 8, i16 8, i16 8, i16 8>
  %3 = or <4 x i16> %1, %2
  ret <4 x i16> %3
}

=== rot_via_fshr
Initial selection DAG: %bb.0 'rot_via_fshr:'
SelectionDAG has 13 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
        t6: v4i16 = BUILD_VECTOR Constant:i16<8>, Constant:i16<8>, Constant:i16<8>, Constant:i16<8>
      t7: v4i16 = rotr t4, t6
    t9: nxv2i16 = insert_subvector undef:nxv2i16, t7, Constant:i64<0>
  t11: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t9
  t12: ch = RISCVISD::RET_GLUE t11, Register:nxv2i16 $v8, t11:1


Optimized lowered selection DAG: %bb.0 'rot_via_fshr:'
SelectionDAG has 11 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
      t13: v4i16 = bswap t4
    t9: nxv2i16 = insert_subvector undef:nxv2i16, t13, Constant:i64<0>
  t11: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t9
  t12: ch = RISCVISD::RET_GLUE t11, Register:nxv2i16 $v8, t11:1

=== rot_via_shift
Initial selection DAG: %bb.0 'rot_via_shift:'
SelectionDAG has 18 nodes:
  t0: ch,glue = EntryToken
    t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
  t4: v4i16 = extract_subvector t2, Constant:i64<0>
    t6: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %1
  t7: v4i16 = extract_subvector t6, Constant:i64<0>
  t9: v4i16 = BUILD_VECTOR Constant:i16<8>, Constant:i16<8>, Constant:i16<8>, Constant:i16<8>
        t10: v4i16 = shl t4, t9
        t11: v4i16 = srl t4, t9
      t12: v4i16 = or t10, t11
    t14: nxv2i16 = insert_subvector undef:nxv2i16, t12, Constant:i64<0>
  t16: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t14
  t17: ch = RISCVISD::RET_GLUE t16, Register:nxv2i16 $v8, t16:1


Optimized lowered selection DAG: %bb.0 'rot_via_shift:'
SelectionDAG has 11 nodes:
  t0: ch,glue = EntryToken
          t2: nxv2i16,ch = CopyFromReg t0, Register:nxv2i16 %0
        t4: v4i16 = extract_subvector t2, Constant:i64<0>
      t19: v4i16 = bswap t4
    t14: nxv2i16 = insert_subvector undef:nxv2i16, t19, Constant:i64<0>
  t16: ch,glue = CopyToReg t0, Register:nxv2i16 $v8, t14
  t17: ch = RISCVISD::RET_GLUE t16, Register:nxv2i16 $v8, t16:1

How ugly would it be to do it as a special case during the shuffle lowering instead?

Another possibility would be a RISCV shuffle to bswap combine before lowering, but having this be a special case in the lowering doesn't seem bad to me.

[luke]

Since these rotates are only emitted in one place during lowering, remove the combine and just
handle it there.

[reames]

LGTM

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

4301

Do we handle FP shuffles here? If so should this be f16 too?

[luke]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

4301

I don't think we can ever get an f16 element type in RotateVT, since it's always going to be larger than the original element type. E.g. i16 has to come from a shuffle of i8s

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

static SDValue lowerVECTOR_SHUFFLEAsRotate(ShuffleVectorSDNode *SVN,
   // DAG.getConstant(RotateAmt, DL, RotateVT)
   //
   // then for e64 we get a weird bitcasted build_vector on RV32 that we're
   // unable to detect as a splat during pattern matching. So directly lower it
   // to a vmv.v.x which gets matched to vror.vi.
   MVT ContainerVT = getContainerForFixedLengthVector(DAG, RotateVT, Subtarget);
   SDValue VL =
       getDefaultVLOps(RotateVT, ContainerVT, DL, DAG, Subtarget).second;
+  SDValue Op = DAG.getBitcast(RotateVT, SVN->getOperand(0));
+
+  SDValue Rotate;
+  // A rotate of an i16 by 8 bits either direction is equivalent to a byteswap,
+  // so canonicalize to vrev8.
+  if (RotateVT.getScalarType() == MVT::i16 && RotateAmt == 8) {
+    Rotate = DAG.getNode(ISD::BSWAP, DL, RotateVT, Op);
+  } else {
     SDValue RotateAmtSplat = DAG.getNode(
         RISCVISD::VMV_V_X_VL, DL, ContainerVT, DAG.getUNDEF(ContainerVT),
         DAG.getConstant(RotateAmt, DL, Subtarget.getXLenVT()), VL);
     RotateAmtSplat =
         convertFromScalableVector(RotateVT, RotateAmtSplat, DAG, Subtarget);
+    Rotate = DAG.getNode(ISD::ROTL, DL, RotateVT, Op, RotateAmtSplat);
+  }
 
-  SDValue Rotate =
-      DAG.getNode(ISD::ROTL, DL, RotateVT,
-                  DAG.getBitcast(RotateVT, SVN->getOperand(0)), RotateAmtSplat);
   return DAG.getBitcast(VT, Rotate);
 }
 
 static SDValue lowerVECTOR_SHUFFLE(SDValue Op, SelectionDAG &DAG,
                                    const RISCVSubtarget &Subtarget) {
   SDValue V1 = Op.getOperand(0);
   SDValue V2 = Op.getOperand(1);
   SDLoc DL(Op);

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-reverse.ll

 ; NO-ZVBB-NEXT:    vslidedown.vi v9, v8, 1
 ; NO-ZVBB-NEXT:    vslideup.vi v9, v8, 1
 ; NO-ZVBB-NEXT:    vmv1r.v v8, v9
 ; NO-ZVBB-NEXT:    ret
 ;
 ; ZVBB-LABEL: reverse_v2i8:
 ; ZVBB:       # %bb.0:
 ; ZVBB-NEXT:    vsetivli zero, 1, e16, mf4, ta, ma
-; ZVBB-NEXT:    vror.vi v8, v8, 8
+; ZVBB-NEXT:    vrev8.v v8, v8
 ; ZVBB-NEXT:    ret
   %res = call <2 x i8> @llvm.experimental.vector.reverse.v2i8(<2 x i8> %a)
   ret <2 x i8> %res
 }
 
 define <4 x i8> @reverse_v4i8(<4 x i8> %a) {
 ; CHECK-LABEL: reverse_v4i8:
 ; CHECK:       # %bb.0:

 ; NO-ZVBB-NEXT:    vslidedown.vi v9, v8, 1
 ; NO-ZVBB-NEXT:    vslideup.vi v9, v8, 1
 ; NO-ZVBB-NEXT:    vmv1r.v v8, v9
 ; NO-ZVBB-NEXT:    ret
 ;
 ; ZVBB-LABEL: reverse_v2i16:
 ; ZVBB:       # %bb.0:
 ; ZVBB-NEXT:    vsetivli zero, 1, e32, mf2, ta, ma
 ; ZVBB-NEXT:    vror.vi v8, v8, 16
 ; ZVBB-NEXT:    ret
   %res = call <2 x i16> @llvm.experimental.vector.reverse.v2i16(<2 x i16> %a)
   ret <2 x i16> %res
 }
 
 define <4 x i16> @reverse_v4i16(<4 x i16> %a) {
 ; CHECK-LABEL: reverse_v4i16:
 ; CHECK:       # %bb.0:

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-shuffle-rotate.ll

 ; CHECK-NEXT:    vle8.v v10, (a0)
 ; CHECK-NEXT:    vrgather.vv v9, v8, v10
 ; CHECK-NEXT:    vmv1r.v v8, v9
 ; CHECK-NEXT:    ret
 ;
 ; ZVBB-V-LABEL: shuffle_v8i8_as_i16:
 ; ZVBB-V:       # %bb.0:
 ; ZVBB-V-NEXT:    vsetivli zero, 4, e16, mf2, ta, ma
-; ZVBB-V-NEXT:    vror.vi v8, v8, 8
+; ZVBB-V-NEXT:    vrev8.v v8, v8
 ; ZVBB-V-NEXT:    ret
 ;
 ; ZVBB-ZVE32X-LABEL: shuffle_v8i8_as_i16:
 ; ZVBB-ZVE32X:       # %bb.0:
 ; ZVBB-ZVE32X-NEXT:    vsetivli zero, 4, e16, m2, ta, ma
-; ZVBB-ZVE32X-NEXT:    vror.vi v8, v8, 8
+; ZVBB-ZVE32X-NEXT:    vrev8.v v8, v8
 ; ZVBB-ZVE32X-NEXT:    ret
   %shuffle = shufflevector <8 x i8> %v, <8 x i8> poison, <8 x i32> <i32 1, i32 0, i32 3, i32 2, i32 5, i32 4, i32 7, i32 6>
   ret <8 x i8> %shuffle
 }
 
 define <8 x i8> @shuffle_v8i8_as_i32_8(<8 x i8> %v) {
 ; CHECK-LABEL: shuffle_v8i8_as_i32_8:
 ; CHECK:       # %bb.0: