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

[Victor Huang via Phabricator via cfe-commits]

NeHuang added a comment.

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



================
Comment at: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll:9
+; RUN:   FileCheck %s
+
+define dso_local <1 x i128> @test_vec_slq(<1 x i128> %a, <1 x i128> %b) #0 {
----------------
nit: please add a description for test purpose. 


================
Comment at: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll:10
+
+define dso_local <1 x i128> @test_vec_slq(<1 x i128> %a, <1 x i128> %b) #0 {
+; CHECK-LABEL: test_vec_slq:
----------------
stefanp wrote:
> nit:
> If you are going to use `#0` you can probably define `attributes #0 = { nounwind }` at the bottom of this file.
nit: The test name and CHECK-LABEL should use `test_vec_vslq`


================
Comment at: llvm/test/CodeGen/PowerPC/p10-vector-shift.ll:10
+
+define dso_local <1 x i128> @test_vec_slq(<1 x i128> %a, <1 x i128> %b) #0 {
+; CHECK-LABEL: test_vec_slq:
----------------
NeHuang wrote:
> stefanp wrote:
> > nit:
> > If you are going to use `#0` you can probably define `attributes #0 = { nounwind }` at the bottom of this file.
> nit: The test name and CHECK-LABEL should use `test_vec_vslq`
+1, if no specific attributes needed. please remove all the `#0` and `#1` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83338