[PowerPC] Fix issue where vsrq is given incorrect shift vector

[stefanp]

Summary

The new Power10 instruction vsrq was being given the wrong shift vector.
The original code assumed that the shift would be found in bits 121 to 127.
This is not correct. The shift is found in bits 57 to 63.
This can be fixed by swaping the first and second double words.

[stefanp]

Forgot to delete the old lines in the TD file.

[nemanjai]

LGTM.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

2561

I know that the issue is not new with P10, but I think it would be good to mention here something like:

// FIXME: The swap is overkill when the shift amount is a constant.
// We should just fix the constant in the DAG.

And then of course, not forget to follow this patch up with one that does that :).

[stefanp]

Added the todo comment in the TD file.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

let Predicates = [IsISA3_1] in {
   def : Pat<(v4i32 (int_ppc_vsx_xxgenpcvwm v4i32:$VRB, imm:$IMM)),
             (v4i32 (COPY_TO_REGCLASS (XXGENPCVWM $VRB, imm:$IMM), VRRC))>;
   def : Pat<(v2i64 (int_ppc_vsx_xxgenpcvdm v2i64:$VRB, imm:$IMM)),
             (v2i64 (COPY_TO_REGCLASS (XXGENPCVDM $VRB, imm:$IMM), VRRC))>;
   def : Pat<(i32 (int_ppc_vsx_xvtlsbb v16i8:$XB, 1)),
             (EXTRACT_SUBREG (XVTLSBB (COPY_TO_REGCLASS $XB, VSRC)), sub_lt)>;
   def : Pat<(i32 (int_ppc_vsx_xvtlsbb v16i8:$XB, 0)),
             (EXTRACT_SUBREG (XVTLSBB (COPY_TO_REGCLASS $XB, VSRC)), sub_eq)>;
-  def : Pat<(v1i128 (shl v1i128:$VRA, v1i128:$VRB)),
-            (v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;
-  def : Pat<(v1i128 (PPCshl v1i128:$VRA, v1i128:$VRB)),
-            (v1i128 (VSLQ v1i128:$VRA, v1i128:$VRB))>;
-  def : Pat<(v1i128 (srl v1i128:$VRA, v1i128:$VRB)),
-            (v1i128 (VSRQ v1i128:$VRA, v1i128:$VRB))>;
-  def : Pat<(v1i128 (PPCsrl v1i128:$VRA, v1i128:$VRB)),
-            (v1i128 (VSRQ v1i128:$VRA, v1i128:$VRB))>;
-  def : Pat<(v1i128 (sra v1i128:$VRA, v1i128:$VRB)),
-            (v1i128 (VSRAQ v1i128:$VRA, v1i128:$VRB))>;
-  def : Pat<(v1i128 (PPCsra v1i128:$VRA, v1i128:$VRB)),
-            (v1i128 (VSRAQ v1i128:$VRA, v1i128:$VRB))>;
 
   def : Pat <(v1i128 (PPClxvrzx xoaddr:$src, 8)),
              (v1i128 (COPY_TO_REGCLASS (LXVRBX xoaddr:$src), VRRC))>;
   def : Pat <(v1i128 (PPClxvrzx xoaddr:$src, 16)),
              (v1i128 (COPY_TO_REGCLASS (LXVRHX xoaddr:$src), VRRC))>;
   def : Pat <(v1i128 (PPClxvrzx xoaddr:$src, 32)),
              (v1i128 (COPY_TO_REGCLASS (LXVRWX xoaddr:$src), VRRC))>;
   def : Pat <(v1i128 (PPClxvrzx xoaddr:$src, 64)),
let AddedComplexity = 400, Predicates = [IsISA3_1, IsLittleEndian] in {
   def : Pat<(store (f32 (extractelt v4f32:$src, 0)), xoaddr:$dst),
             (STXVRWX $src, xoaddr:$dst)>;
   def : Pat<(store (i64 (extractelt v2i64:$src, 0)), xoaddr:$dst),
             (STXVRDX $src, xoaddr:$dst)>;
   def : Pat<(store (f64 (extractelt v2f64:$src, 0)), xoaddr:$dst),
             (STXVRDX $src, xoaddr:$dst)>;
  }
 
+// FIXME: The swap is overkill when the shift amount is a constant.
+// We should just fix the constant in the DAG.
+let AddedComplexity = 400, Predicates = [IsISA3_1, HasVSX] in {
+  def : Pat<(v1i128 (shl v1i128:$VRA, v1i128:$VRB)),
+            (v1i128 (VSLQ v1i128:$VRA,
+                     (XXPERMDI (COPY_TO_REGCLASS $VRB, VSRC),
+                               (COPY_TO_REGCLASS $VRB, VSRC), 2)))>;
+  def : Pat<(v1i128 (PPCshl v1i128:$VRA, v1i128:$VRB)),
+            (v1i128 (VSLQ v1i128:$VRA,
+                     (XXPERMDI (COPY_TO_REGCLASS $VRB, VSRC),
+                               (COPY_TO_REGCLASS $VRB, VSRC), 2)))>;
+  def : Pat<(v1i128 (srl v1i128:$VRA, v1i128:$VRB)),
+            (v1i128 (VSRQ v1i128:$VRA,
+                     (XXPERMDI (COPY_TO_REGCLASS $VRB, VSRC),
+                               (COPY_TO_REGCLASS $VRB, VSRC), 2)))>;
+  def : Pat<(v1i128 (PPCsrl v1i128:$VRA, v1i128:$VRB)),
+            (v1i128 (VSRQ v1i128:$VRA,
+                     (XXPERMDI (COPY_TO_REGCLASS $VRB, VSRC),
+                               (COPY_TO_REGCLASS $VRB, VSRC), 2)))>;
+  def : Pat<(v1i128 (sra v1i128:$VRA, v1i128:$VRB)),
+            (v1i128 (VSRAQ v1i128:$VRA,
+                     (XXPERMDI (COPY_TO_REGCLASS $VRB, VSRC),
+                               (COPY_TO_REGCLASS $VRB, VSRC), 2)))>;
+  def : Pat<(v1i128 (PPCsra v1i128:$VRA, v1i128:$VRB)),
+            (v1i128 (VSRAQ v1i128:$VRA,
+                     (XXPERMDI (COPY_TO_REGCLASS $VRB, VSRC),
+                               (COPY_TO_REGCLASS $VRB, VSRC), 2)))>;
+}
+
 class xxevalPattern <dag pattern, bits<8> imm> :
   Pat<(v4i32 pattern), (XXEVAL $vA, $vB, $vC, imm)> {}
 
 let AddedComplexity = 400, Predicates = [PrefixInstrs] in {
  def : Pat<(v4i32 (build_vector i32immNonAllOneNonZero:$A,
                                 i32immNonAllOneNonZero:$A,
                                 i32immNonAllOneNonZero:$A,
                                 i32immNonAllOneNonZero:$A)),

llvm/test/CodeGen/PowerPC/p10-vector-shift.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
 ; RUN:   FileCheck %s
 
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
 ; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
 ; RUN:   FileCheck %s
 
 ; These test cases demonstrate that the vector shift quadword instructions
 ; introduced within Power10 are correctly exploited.
 
 define dso_local <1 x i128> @test_vec_vslq(<1 x i128> %a, <1 x i128> %b) {
 ; CHECK-LABEL: test_vec_vslq:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xxswapd v3, v3
 ; CHECK-NEXT:    vslq v2, v2, v3
 ; CHECK-NEXT:    blr
 entry:
   %rem = urem <1 x i128> %b, <i128 128>
   %shl = shl <1 x i128> %a, %rem
   ret <1 x i128> %shl
 }
 
 define dso_local <1 x i128> @test_vec_vsrq(<1 x i128> %a, <1 x i128> %b) {
 ; CHECK-LABEL: test_vec_vsrq:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xxswapd v3, v3
 ; CHECK-NEXT:    vsrq v2, v2, v3
 ; CHECK-NEXT:    blr
 entry:
   %rem = urem <1 x i128> %b, <i128 128>
   %shr = lshr <1 x i128> %a, %rem
   ret <1 x i128> %shr
 }
 
 define dso_local <1 x i128> @test_vec_vsraq(<1 x i128> %a, <1 x i128> %b) {
 ; CHECK-LABEL: test_vec_vsraq:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xxswapd v3, v3
 ; CHECK-NEXT:    vsraq v2, v2, v3
 ; CHECK-NEXT:    blr
 entry:
   %rem = urem <1 x i128> %b, <i128 128>
   %shr = ashr <1 x i128> %a, %rem
   ret <1 x i128> %shr
 }
 
 define dso_local <1 x i128> @test_vec_vslq2(<1 x i128> %a, <1 x i128> %b) {
 ; CHECK-LABEL: test_vec_vslq2:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xxswapd v3, v3
 ; CHECK-NEXT:    vslq v2, v2, v3
 ; CHECK-NEXT:    blr
 entry:
   %shl = shl <1 x i128> %a, %b
   ret <1 x i128> %shl
 }
 
 define dso_local <1 x i128> @test_vec_vsrq2(<1 x i128> %a, <1 x i128> %b) {
 ; CHECK-LABEL: test_vec_vsrq2:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xxswapd v3, v3
 ; CHECK-NEXT:    vsrq v2, v2, v3
 ; CHECK-NEXT:    blr
 entry:
   %shr = lshr <1 x i128> %a, %b
   ret <1 x i128> %shr
 }
 
 define dso_local <1 x i128> @test_vec_vsraq2(<1 x i128> %a, <1 x i128> %b) {
 ; CHECK-LABEL: test_vec_vsraq2:
 ; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    xxswapd v3, v3
 ; CHECK-NEXT:    vsraq v2, v2, v3
 ; CHECK-NEXT:    blr
 entry:
   %shr = ashr <1 x i128> %a, %b
   ret <1 x i128> %shr
 }