[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

nit: indentation issue

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

919

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

17099

/* 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

17099

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

593 ↗

(On Diff #277896)

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

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

917–923

nit: remove extra space after <

918

Unrelated space change?

919

nit: remove after space before ,

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

10 ↗

(On Diff #277896)

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

[amyk]

clang/lib/Headers/altivec.h

17101

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

17106

nit:
Is this supposed to be vec_slq?

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

10 ↗

(On Diff #278235)

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

41 ↗

(On Diff #278235)

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

9 ↗

(On Diff #278235)

nit: please add a description for test purpose.

10 ↗

(On Diff #278235)

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

10 ↗

(On Diff #278235)

+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

17099

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.

17106

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

17099

Add a space after this comment.

17109

Please remove any commented code.

17125

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

llvm/lib/Target/PowerPC/PPCISelLowering.cpp

1100 ↗

(On Diff #281783)

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

936

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

939

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 ↗

(On Diff #281783)

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

939

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

17125

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

10 ↗

(On Diff #283980)

nit:

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

clang/include/clang/Basic/BuiltinsPPC.def

 BUILTIN(__builtin_altivec_vrlh, "V8sV8sV8Us", "")
 BUILTIN(__builtin_altivec_vrlw, "V4iV4iV4Ui", "")
 BUILTIN(__builtin_altivec_vrld, "V2LLiV2LLiV2ULLi", "")
 
 BUILTIN(__builtin_altivec_vsel_4si, "V4iV4iV4iV4Ui", "")
 
 BUILTIN(__builtin_altivec_vsl, "V4iV4iV4i", "")
 BUILTIN(__builtin_altivec_vslo, "V4iV4iV4i", "")
+BUILTIN(__builtin_altivec_vslq, "V1ULLLiV1ULLLiV1ULLLi", "")
 
 BUILTIN(__builtin_altivec_vsrab, "V16cV16cV16Uc", "")
 BUILTIN(__builtin_altivec_vsrah, "V8sV8sV8Us", "")
 BUILTIN(__builtin_altivec_vsraw, "V4iV4iV4Ui", "")
+BUILTIN(__builtin_altivec_vsraq, "V1ULLLiV1ULLLiV1ULLLi", "")
 
 BUILTIN(__builtin_altivec_vsr, "V4iV4iV4i", "")
 BUILTIN(__builtin_altivec_vsro, "V4iV4iV4i", "")
+BUILTIN(__builtin_altivec_vsrq, "V1ULLLiV1ULLLiV1ULLLi", "")
 
 BUILTIN(__builtin_altivec_vrfin, "V4fV4f", "")
 
 BUILTIN(__builtin_altivec_vrsqrtefp, "V4fV4f", "")
 
 BUILTIN(__builtin_altivec_vsubcuw, "V4UiV4UiV4Ui", "")
 
 BUILTIN(__builtin_altivec_vsum4sbs, "V4SiV16ScV4Si", "")

clang/lib/Headers/altivec.h

 }
 
 static __inline__ vector double __ATTRS_o_ai
 vec_blendv(vector double __a, vector double __b,
            vector unsigned long long __c) {
   return __builtin_vsx_xxblendvd(__a, __b, __c);
 }
 #endif /* __VSX__ */
+
+/* vector shifts for quadwords */
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sl(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __builtin_altivec_vsl(__a, __b);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sl(vector unsigned __int128 __a, vector signed __int128 __b) {
+  return __builtin_altivec_vsl(__a, (vector unsigned __int128) __b);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sr(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __builtin_altivec_vsr(__a, __b);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sr(vector unsigned __int128 __a, vector signed __int128 __b) {
+  return __builtin_altivec_vsr(__a, (vector unsigned __int128) __b);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sra(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return __builtin_altivec_vsraq(__a, __b);
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_sra(vector unsigned __int128 __a, vector signed __int128 __b) {
+  return __builtin_altivec_vsraq(__a, (vector unsigned __int128) __b);
+}
 #endif /* __POWER10_VECTOR__ */
 
 #undef __ATTRS_o_ai
 
 #endif /* __ALTIVEC_H */

llvm/include/llvm/IR/IntrinsicsPowerPC.td

 def int_ppc_altivec_vsl   : PowerPC_Vec_WWW_Intrinsic<"vsl">;
 def int_ppc_altivec_vslo  : PowerPC_Vec_WWW_Intrinsic<"vslo">;
 
 def int_ppc_altivec_vslb  : PowerPC_Vec_BBB_Intrinsic<"vslb">;
 def int_ppc_altivec_vslv  : PowerPC_Vec_BBB_Intrinsic<"vslv">;
 def int_ppc_altivec_vsrv  : PowerPC_Vec_BBB_Intrinsic<"vsrv">;
 def int_ppc_altivec_vslh  : PowerPC_Vec_HHH_Intrinsic<"vslh">;
 def int_ppc_altivec_vslw  : PowerPC_Vec_WWW_Intrinsic<"vslw">;
+def int_ppc_altivec_vslq  : PowerPC_Vec_QQQ_Intrinsic<"vslq">;
 
 // Right Shifts.
 def int_ppc_altivec_vsr   : PowerPC_Vec_WWW_Intrinsic<"vsr">;
 def int_ppc_altivec_vsro  : PowerPC_Vec_WWW_Intrinsic<"vsro">;
 
 def int_ppc_altivec_vsrb  : PowerPC_Vec_BBB_Intrinsic<"vsrb">;
 def int_ppc_altivec_vsrh  : PowerPC_Vec_HHH_Intrinsic<"vsrh">;
 def int_ppc_altivec_vsrw  : PowerPC_Vec_WWW_Intrinsic<"vsrw">;
+def int_ppc_altivec_vsrq   : PowerPC_Vec_QQQ_Intrinsic<"vsrq">;
 def int_ppc_altivec_vsrab : PowerPC_Vec_BBB_Intrinsic<"vsrab">;
 def int_ppc_altivec_vsrah : PowerPC_Vec_HHH_Intrinsic<"vsrah">;
 def int_ppc_altivec_vsraw : PowerPC_Vec_WWW_Intrinsic<"vsraw">;
+def int_ppc_altivec_vsraq : PowerPC_Vec_QQQ_Intrinsic<"vsraq">;
 
 // Rotates.
 def int_ppc_altivec_vrlb  : PowerPC_Vec_BBB_Intrinsic<"vrlb">;
 def int_ppc_altivec_vrlh  : PowerPC_Vec_HHH_Intrinsic<"vrlh">;
 def int_ppc_altivec_vrlw  : PowerPC_Vec_WWW_Intrinsic<"vrlw">;
 def int_ppc_altivec_vrld  : PowerPC_Vec_DDD_Intrinsic<"vrld">;
 
 let TargetPrefix = "ppc" in {  // All PPC intrinsics start with "llvm.ppc.".

llvm/lib/Target/PowerPC/PPCInstrPrefix.td

XForm_XT6_IMM5_VB5<60, 949, (outs vsrc:$XT), (ins vrrc:$VRB, s5imm:$IMM),
                         "xxgenpcvdm $XT, $VRB, $IMM", IIC_VecGeneral, []>;
    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 VSLQ   : VX1_Int_Ty< 261, "vslq", int_ppc_altivec_vslq, v1i128>;
+  def VSRAQ  : VX1_Int_Ty< 773, "vsraq", int_ppc_altivec_vsraq, v1i128>;
+  def VSRQ   : VX1_Int_Ty< 517, "vsrq" , int_ppc_altivec_vsrq, v1i128>;
+
 }
 
 //---------------------------- 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))>;
 }
 
 let AddedComplexity = 400, Predicates = [PrefixInstrs] in {
  def : Pat<(v4i32 (build_vector i32immNonAllOneNonZero:$A,
                                 i32immNonAllOneNonZero:$A,
                                 i32immNonAllOneNonZero:$A,
                                 i32immNonAllOneNonZero:$A)),
            (v4i32 (XXSPLTIW imm:$A))>;
  def : Pat<(f32 nzFPImmAsi32:$A),
            (COPY_TO_REGCLASS (XXSPLTIDP (getFPAs32BitInt fpimm:$A)),
                              VSFRC)>;
  def : Pat<(f64 nzFPImmAsi32:$A),
            (COPY_TO_REGCLASS (XXSPLTIDP (getFPAs32BitInt fpimm:$A)),

llvm/test/MC/Disassembler/PowerPC/ppc64-encoding-ISA31.txt

 # CHECK: vinswrx 1, 2, 3
 0x10 0x22 0x1b 0x8f
 
 # CHECK: vinsdlx 1, 2, 3
 0x10 0x22 0x1a 0xcf
 
 # CHECK: vinsdrx 1, 2, 3
 0x10 0x22 0x1b 0xcf
+
+# CHECK: vsrq 2, 3, 4
+0x10 0x43 0x22 0x05
+
+# CHECK: vslq 2, 3, 4
+0x10 0x43 0x21 0x05
+
+# CHECK: vsraq 2, 3, 4
+0x10 0x43 0x23 0x05

llvm/test/MC/PowerPC/ppc64-encoding-ISA31.s

 # CHECK-LE: vinswrx 1, 2, 3                       # encoding: [0x8f,0x1b,0x22,0x10]
             vinswrx 1, 2, 3
 # CHECK-BE: vinsdlx 1, 2, 3                       # encoding: [0x10,0x22,0x1a,0xcf]
 # CHECK-LE: vinsdlx 1, 2, 3                       # encoding: [0xcf,0x1a,0x22,0x10]
             vinsdlx 1, 2, 3
 # CHECK-BE: vinsdrx 1, 2, 3                       # encoding: [0x10,0x22,0x1b,0xcf]
 # CHECK-LE: vinsdrx 1, 2, 3                       # encoding: [0xcf,0x1b,0x22,0x10]
             vinsdrx 1, 2, 3
+# CHECK-BE: vslq 2, 3, 4                          # encoding: [0x10,0x43,0x21,0x05]
+# CHECK-LE: vslq 2, 3, 4                          # encoding: [0x05,0x21,0x43,0x10]
+            vslq 2, 3, 4
+# CHECK-BE: vsraq 2, 3, 4                         # encoding: [0x10,0x43,0x23,0x05]
+# CHECK-LE: vsraq 2, 3, 4                         # encoding: [0x05,0x23,0x43,0x10]
+            vsraq 2, 3, 4
+# CHECK-BE: vsrq 2, 3, 4                          # encoding: [0x10,0x43,0x22,0x05]
+# CHECK-LE: vsrq 2, 3, 4                          # encoding: [0x05,0x22,0x43,0x10]
+            vsrq 2, 3, 4