[PowerPC][Power10] Implemented Vector Shift Builtins

[Conanap]

Summary

Implemented Builtins for vector right and left shift along with appropriate test cases for ISA3.1. Depends on D83516.

[bsaleil]

Shouldn't we have test cases to test vec_sl, vec_sr and vec_sra ?

llvm/include/llvm/IR/IntrinsicsPowerPC.td

800 ↗

(On Diff #276179)

nit: indentation issue

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1009

nit: extra spaces before : here and in the next two lines

[amyk]

This will need to be rebased against your 2608 instruction definitions patch.
But yes, I believe you are missing the clang and llc test case for this patch. Requesting changes due to missing tests.

clang/lib/Headers/altivec.h

17217

/* vs[l | r | raq] */ (with a new line after the comment)

[Conanap]

Added tests, added extra comments.

[Conanap]

Also removed instr def as it will be part of D83516.

[amyk]

clang/lib/Headers/altivec.h

17217

I think we can remove /* vector shifts for quadwords */.
Then, we can add a new line after /* vs[l | r | raq] */ for consistency of the comments.

clang/test/CodeGen/builtins-ppc-p10vector.c

586

vector unsigned you mean? For the return of this test and every other unsigned test case.

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1113–1114

nit: remove extra space after <

1115

nit: remove after space before ,

1121

Unrelated space change?

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

11

Can probably remove these globals as you are not using them in your test.

[amyk]

clang/lib/Headers/altivec.h

17219

I believe there are supposed to be signed and unsigned versions of the functions? It looks like you currently have all of them as unsigned.

[Conanap]

Formatting fixes, fixed test case return type, updated builtins' signatures to correct signatures.

[stefanp]

Just a few nits for this patch.

Edit: Please wait for Amy to give the approval.

clang/lib/Headers/altivec.h

17224

nit:
Is this supposed to be vec_slq?

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

11

nit:
If you are going to use #0 you can probably define attributes #0 = { nounwind } at the bottom of this file.

42

nit:
You probably don't need the #1 as it is not defined anyway.

[NeHuang]

Overall seems fine to me, but of course, please wait to hear from Amy.
Just some nits for the test case.

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

10

nit: please add a description for test purpose.

11

nit: The test name and CHECK-LABEL should use test_vec_vslq

11

+1, if no specific attributes needed. please remove all the #0 and #1

[amyk]

Please also change the function names.

clang/lib/Headers/altivec.h

17217

Actually, sorry, I think this comment should be the following instead:
/* vec_s[l | r | ra] */ since these functions are actually supposed to be vec_sl, vec_sr, vec_sra.

17224

Actually vec_sl seems to be correct. However, that would mean the other functions need to be renamed as the functions are supposed to be: vec_sl, vec_sr, vec_sra. Albion, could you please rename these functions and also ensure your test uses the correct naming.

[Conanap]

Fixed function names, test case clean up

[amyk]

I realize it may be possible to open code these, as these functions already exist in altivec.h. Could you look into if this is the case?

[Conanap]

Converted the impelmentation to an open coded implementation and updated the test cases as appropriate.

[lei]

Please address the auto generated clang-format issues for the added code in this patch.

[Conanap]

Fixed formatting based on bot feedback.

[Conanap]

Replaced a forgotten test file.

[amyk]

clang/lib/Headers/altivec.h

17217

Add a space after this comment.

17227

Please remove any commented code.

17243

Could you please fix the indentation of the returns to make them all consistent?

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1100

No brackets are needed here.

Also, I think it might make sense to move this block into the previous hasP9Altivec condition since in there it has:

setOperationAction(ISD::SHL, MVT::v1i128, Legal);
setOperationAction(ISD::SRL, MVT::v1i128, Legal);

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1141

There is no need to make a new predicate block, you can put these anonymous patterns in the block above.

1144

I noticed that we have patterns for the PPCISD nodes, but I think no tests to show these patterns?

[amyk]

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1100

Oops, I made that comment above since I thought you were putting the block inside a hasP9Altivec block. Sorry.

Maybe you can move this condition right underneath the hasP9Altivec block that contains the SHL/SRL to be closer to those instructions.

[amyk]

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

1144

My apologies, upon closer inspection I noticed that your tests are in fact the ones with the PPCISD nodes but you are missing the regular shl/srl/sra. Please add the tests for these, as well.

[Conanap]

Added shl tests, formatting fixes

clang/lib/Headers/altivec.h

17243

I ran clang-format this time, lmk if we should change this as it doesn't look like clang-format was too consistent with itself

[amyk]

Thanks for addressing the comments. LGTM.

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

11

nit:

These test cases demonstrate that the vector shift quadword instructions introduced 
within Power10 are correctly exploited.

clang/lib/Headers/altivec.h

 
 /* vec_test_lsbb_all_zeros */
 
 static __inline__ int __ATTRS_o_ai
 vec_test_lsbb_all_zeros(vector unsigned char __a) {
   return __builtin_vsx_xvtlsbb(__a, 0);
 }
 #endif /* __VSX__ */
+
+/* vs[l | r | ra] */
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sl(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __a << (__b %
+                 (vector unsigned __int128)(sizeof(unsigned __int128) *
+                                           __CHAR_BIT__));
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sl(vector signed __int128 __a, vector unsigned __int128 __b) {
+  // return (vector signed __int128)vec_sl((vector unsigned __int128)__a, __b);
+  return __a << (__b %
+                 (vector unsigned __int128)(sizeof(unsigned __int128) *
+                                           __CHAR_BIT__));
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sr(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __a >> (__b %
+                 (vector unsigned __int128)(sizeof(unsigned __int128) *
+                                           __CHAR_BIT__));
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sr(vector signed __int128 __a, vector unsigned __int128 __b) {
+  // return (vector signed __int128)vec_sr((vector unsigned __int128)__a, __b);
+  return (vector signed __int128)(
+            ((vector unsigned __int128) __a) >>
+            (__b % (vector unsigned __int128)(sizeof(
+                                                  unsigned __int128) *
+                                             __CHAR_BIT__)));
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sra(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  // return (vector unsigned __int128)vec_sra((vector signed __int128)__a, __b);
+  return (vector unsigned __int128) (((vector signed __int128)__a) >> (__b %
+                 (vector unsigned __int128)(sizeof(
+                                                unsigned __int128) *
+                                           __CHAR_BIT__)));
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_sra(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return __a >> (__b %
+                 (vector unsigned __int128)(sizeof(unsigned __int128) *
+                                            __CHAR_BIT__));
+}
+
 #endif /* __POWER10_VECTOR__ */
 
 #undef __ATTRS_o_ai
 
 #endif /* __ALTIVEC_H */

clang/test/CodeGen/builtins-ppc-p10vector.c

 vector signed char vsca, vscb;
 vector unsigned char vuca, vucb, vucc;
 vector signed short vssa, vssb;
 vector unsigned short vusa, vusb, vusc;
 vector signed int vsia, vsib;
 vector unsigned int vuia, vuib, vuic;
 vector signed long long vslla, vsllb;
 vector unsigned long long vulla, vullb, vullc;
++vector signed __int128 vi128a;
 vector unsigned __int128 vui128a, vui128b, vui128c;
 vector float vfa, vfb;
 vector double vda, vdb;
 unsigned int uia, uib;
 unsigned char uca;
 unsigned short usa;
 unsigned long long ulla;
 

 
 void test_vec_xst_trunc_sc(vector signed __int128 __a, signed long long __b,
                            signed char *__c) {
   // CHECK: store i8 %{{.+}}, i8* %{{.+}}, align 1
   vec_xst_trunc(__a, __b, __c);
 }
 
 void test_vec_xst_trunc_uc(vector unsigned __int128 __a, signed long long __b,
                            unsigned char *__c) {
   // CHECK: store i8 %{{.+}}, i8* %{{.+}}, align 1
   vec_xst_trunc(__a, __b, __c);
 }
 
 void test_vec_xst_trunc_ss(vector signed __int128 __a, signed long long __b,
                            signed short *__c) {
   // CHECK: store i16 %{{.+}}, i16* %{{.+}}, align 2
   vec_xst_trunc(__a, __b, __c);
int test_vec_test_lsbb_all_ones(void) {
   return vec_test_lsbb_all_ones(vuca);
 }
 
 int test_vec_test_lsbb_all_zeros(void) {
   // CHECK: @llvm.ppc.vsx.xvtlsbb(<16 x i8> %{{.+}}, i32 0
   // CHECK-NEXT: ret i32
   return vec_test_lsbb_all_zeros(vuca);
 }
+
+vector unsigned __int128 test_vec_slq_unsigned (void) {
+  // CHECK-LABEL: test_vec_slq_unsigned
+  // CHECK: shl <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128> %{{.+}}
+  return vec_sl(vui128a, vui128b);
+}
+
+vector signed __int128 test_vec_slq_signed (void) {
+  // CHECK-LABEL: test_vec_slq_signed
+  // CHECK: shl <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sl(vi128a, vui128a);
+}
+
+vector unsigned __int128 test_vec_srq_unsigned (void) {
+  // CHECK-LABEL: test_vec_srq_unsigned
+  // CHECK: lshr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sr(vui128a, vui128b);
+}
+
+vector signed __int128 test_vec_srq_signed (void) {
+  // CHECK-LABEL: test_vec_srq_signed
+  // CHECK: lshr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sr(vi128a, vui128a);
+}
+
+vector unsigned __int128 test_vec_sraq_unsigned (void) {
+  // CHECK-LABEL: test_vec_sraq_unsigned
+  // CHECK: ashr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sra(vui128a, vui128b);
+}
+
+vector signed __int128 test_vec_sraq_signed (void) {
+  // CHECK-LABEL: test_vec_sraq_signed
+  // CHECK: ashr <1 x i128> %{{.+}}, %{{.+}}
+  // CHECK: ret <1 x i128>
+  return vec_sra(vi128a, vui128a);
+}

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

if (Subtarget.hasP9Altivec()) {
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v4i8,  Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v4i16, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v4i32, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i8,  Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i16, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i32, Legal);
       setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::v2i64, Legal);
     }
+
+    if (Subtarget.isISA3_1()) {
+      setOperationAction(ISD::SRA, MVT::v1i128, Legal);
+    }
   }
 
   if (Subtarget.has64BitSupport())
     setOperationAction(ISD::PREFETCH, MVT::Other, Legal);
 
   setOperationAction(ISD::READCYCLECOUNTER, MVT::i64, isPPC64 ? Legal : Custom);
 
   if (!isPPC64) {

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

def VCLRLB : VXForm_1<397, (outs vrrc:$vD), (ins vrrc:$vA, gprc:$rB),
                          "vclrlb $vD, $vA, $rB", IIC_VecGeneral,
                          [(set v16i8:$vD,
                                (int_ppc_altivec_vclrlb v16i8:$vA, i32:$rB))]>;
    def VCLRRB : VXForm_1<461, (outs vrrc:$vD), (ins vrrc:$vA, gprc:$rB),
                          "vclrrb $vD, $vA, $rB", IIC_VecGeneral,
                          [(set v16i8:$vD,
                                (int_ppc_altivec_vclrrb v16i8:$vA, i32:$rB))]>;
   def VMULLD : VXForm_1<457, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
                         "vmulld $vD, $vA, $vB", IIC_VecGeneral,
                         [(set v2i64:$vD, (mul v2i64:$vA, v2i64:$vB))]>;
   def VMULHSW : VXForm_1<905, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
                          "vmulhsw $vD, $vA, $vB", IIC_VecGeneral,
                          [(set v4i32:$vD, (mulhs v4i32:$vA, v4i32:$vB))]>;
   def VMULHUW : VXForm_1<649, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
                          "vmulhuw $vD, $vA, $vB", IIC_VecGeneral,
                          [(set v4i32:$vD, (mulhu v4i32:$vA, v4i32:$vB))]>;
   def VMULHSD : VXForm_1<969, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
let Predicates = [IsISA3_1] in {
   def VCMPUQ : VXForm_BF3_VAB5<257, (outs crrc:$BF), (ins vrrc:$vA, vrrc:$vB),
                                "vcmpuq $BF, $vA, $vB", IIC_VecGeneral, []>;
   def VCMPSQ : VXForm_BF3_VAB5<321, (outs crrc:$BF), (ins vrrc:$vA, vrrc:$vB),
                                "vcmpsq $BF, $vA, $vB", IIC_VecGeneral, []>;
   def VRLQNM : VX1_VT5_VA5_VB5<325, "vrlqnm", []>;
   def VRLQMI : VXForm_1<69, (outs vrrc:$vD),
                         (ins vrrc:$vA, vrrc:$vB, vrrc:$vDi),
                         "vrlqmi $vD, $vA, $vB", IIC_VecFP, []>,
                         RegConstraint<"$vDi = $vD">, NoEncode<"$vDi">;
   def VSLQ : VX1_VT5_VA5_VB5<261, "vslq", []>;
   def VSRAQ : VX1_VT5_VA5_VB5<773, "vsraq", []>;
   def VSRQ : VX1_VT5_VA5_VB5<517, "vsrq", []>;
   def VRLQ : VX1_VT5_VA5_VB5<5, "vrlq", []>;
   def XSCVQPUQZ : X_VT5_XO5_VB5<63, 0, 836, "xscvqpuqz", []>;
   def XSCVQPSQZ : X_VT5_XO5_VB5<63, 8, 836, "xscvqpsqz", []>;
   def XSCVUQQP : X_VT5_XO5_VB5<63, 3, 836, "xscvuqqp", []>;
   def XSCVSQQP : X_VT5_XO5_VB5<63, 11, 836, "xscvsqqp", []>;
 }
 
 //---------------------------- Anonymous Patterns ----------------------------//
 let Predicates = [IsISA3_1] in {
   def : Pat<(v16i8 (int_ppc_vsx_xxgenpcvbm v16i8:$VRB, imm:$IMM)),
             (v16i8 (COPY_TO_REGCLASS (XXGENPCVBM $VRB, imm:$IMM), VRRC))>;
   def : Pat<(v8i16 (int_ppc_vsx_xxgenpcvhm v8i16:$VRB, imm:$IMM)),
             (v8i16 (COPY_TO_REGCLASS (XXGENPCVHM $VRB, imm:$IMM), VRRC))>;
   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)>;
 }
 
+/* Vector shifts for ISA3_1 */
+let Predicates = [IsISA3_1] in {
+  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))>;
+}
+
 let AddedComplexity = 400, Predicates = [IsISA3_1] in {
   def : Pat<(truncstorei8 (i32 (vector_extract v16i8:$rS, 0)), xoaddr:$src),
             (STXVRBX (COPY_TO_REGCLASS $rS, VSRC), xoaddr:$src)>;
   def : Pat<(truncstorei16 (i32 (vector_extract v8i16:$rS, 0)), xoaddr:$src),
             (STXVRHX (COPY_TO_REGCLASS $rS, VSRC), xoaddr:$src)>;
   def : Pat<(store (i32 (vector_extract v4i32:$rS, 0)), xoaddr:$src),
             (STXVRWX (COPY_TO_REGCLASS $rS, VSRC), xoaddr:$src)>;
   def : Pat<(store (i64 (vector_extract v2i64:$rS, 0)), xoaddr:$src),

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 tests ensure that vector shift quadword builtins are correctly
+; exploited and selected for during codeGen.
+
+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:    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:    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:    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
+}
+