[PATCH] D83338: [PowerPC][Power10] Implemented Vector Shift Builtins

[Amy Kwan via Phabricator via llvm-commits]

amyk added inline comments.


================
Comment at: clang/lib/Headers/altivec.h:17151
+
+/* vector shifts for quadwords */
+/* vs[l | r | raq] */
----------------
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.


================
Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:593
+
+vector signed __int128 test_vec_slq_signed (void) {
+  // CHECK-LABEL: test_vec_slq_signed
----------------
`vector unsigned` you mean? For the return of this test and every other `unsigned` test case.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1030
                         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 VSLQ  : VX1_Int_Ty< 261, "vslq", int_ppc_altivec_vslq, v1i128>;
+  def VSRAQ : VX1_Int_Ty< 773, "vsraq", int_ppc_altivec_vsraq, v1i128>;
----------------
nit: remove extra space after `<`


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1032
+  def VSRAQ : VX1_Int_Ty< 773, "vsraq", int_ppc_altivec_vsraq, v1i128>;
+  def VSRQ  : VX1_Int_Ty< 517, "vsrq" , int_ppc_altivec_vsrq, v1i128>;
   def VRLQ   : VX1_VT5_VA5_VB5<5, "vrlq", []>;
----------------
nit: remove after space before `,`


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1039
   def XSCVSQQP  : X_VT5_XO5_VB5<63, 11, 836, "xscvsqqp", []>;
+
 }
----------------
Unrelated space change?


================
Comment at: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll:10
+
+ at vui128a = dso_local global <1 x i128> zeroinitializer, align 16
+ at vui128b = dso_local global <1 x i128> zeroinitializer, align 16
----------------
Can probably remove these globals as you are not using them in your test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338