[PPC] Implement vector reverse elements builtins (vec_reve)

[jtony]

Summary

This patch introduces the following builtins:

vector bool char vec_reve (vector bool char);
vector signed char vec_reve (vector signed char);
vector unsigned char vec_reve (vector unsigned char);
vector bool int vec_reve (vector bool int);
vector signed int vec_reve (vector signed int);
vector unsigned int vec_reve (vector unsigned int);
vector bool long long vec_reve (vector bool long long);
vector signed long long vec_reve (vector signed long long);
vector unsigned long long vec_reve (vector unsigned long long);
vector bool short vec_reve (vector bool short);
vector signed short vec_reve (vector signed short);
vector unsigned short vec_reve (vector unsigned short);
vector double vec_reve (vector double);
vector float vec_reve (vector float);

[nemanjai]

lib/Headers/altivec.h

15041

I believe the indentation is off on all of these. Please use clang-format or use the plugin/config for your favourite editor (from the utils/ directory). Both of those are ways to enforce the indentation portion of the coding guidelines.

test/CodeGen/builtins-ppc-altivec.c

27

Please do not add conditionally compiled code into the test case. This can lead to confusing results. See my comment below.

9000

Please provide CHECK-LABEL for all the check prefixes and keep the checks upper case (I'm not sure if this actually performs the LABEL check).

9003

Don't check the name of the variable. Just start the check pattern with the instruction (shufflevector).

9042

It would be difficult for an uninitiated reader to decipher what is actually being tested here as a result of this condition.
For example, none of the "CHECK" lines can succeed here because the run commands they correspond to do not include VSX. I'm actually surprised this test case passes.

The functions whose definitions require VSX should go into a separate file in which the run commands will include VSX (for example builtins-ppc-vsx.c).

[nemanjai]

test/CodeGen/builtins-ppc-altivec.c

9042

Oh, I see why it's passing. None of these checks are actually performed. FileCheck will perform the check if the line starts with (in this case):
CHECK:
CHECK-LE:

i.e. Notice the colon at the end.

[nemanjai]

test/CodeGen/builtins-ppc-altivec.c

9000

I just realized that the existing tests in this file don't check the label for both prefixes. That's fine and you're not expected to change that, but since you're writing a new test, provide both
CHECK-LABEL:
CHECK-LE-LABEL:

[amehsan]

test/CodeGen/builtins-ppc-altivec.c

9000

Something that I do sometimes to make sure my test is actually doing something is the following. I change the testcase slightly so it has to fail. Then I check again. If it passes, then something is wrong.

[amehsan]

test/CodeGen/builtins-ppc-altivec.c

9000

I think this comment should have been further down in the file :P

[jtony]

Modify the code according to NEMANJA IVANOVIC and EHSAN AMIRI's comments (fix clang format violation and move some VSX specific source to builtins-ppc-vsx.c etc.) .

[nemanjai]

Other than the inline comment, this LGTM but I'll let @kbarton give the official approval so we get another pair of eyes on the patch.

lib/Headers/altivec.h

15041

Sorry I missed this in the first review, but why do we have a temp here? Why not just a return statement to return the result of __builtin_shufflevector? I realize that it won't make much of a difference with the code-gen with optimization, but it is clearer and more readable to just have a return statement since the temp isn't used for anything else.

[jtony]

Remove unnecessary temp variables.

[kbarton]

LGTM

[echristo]

One small minor thing and then LGTM. Thanks!

test/CodeGen/builtins-ppc-altivec.c

9001

Can you sink these to just inside the function? (Minor preference)

[jtony]

Sink CHECK-LABEL andCHECK-LE-LABEL to just inside the function test8 according to echristo's comment.

[nemanjai]

Committed revision 285218 on behalf of Tony.
Tony, please observe the buildbots at http://lab.llvm.org:8011/console to see if you spot any failures. If none, please close this review.

[nemanjai]

Committed revision 285218 on behalf of Tony.
Tony, please observe the buildbots at http://lab.llvm.org:8011/console to see if you spot any failures. If none, please close this review.

[jtony]

In D25906#580017, @nemanjai wrote:

Committed revision 285218 on behalf of Tony.
Tony, please observe the buildbots at http://lab.llvm.org:8011/console to see if you spot any failures. If none, please close this review.

Sure, I will monitor the console result. Thanks!

[nemanjai]

This is ready to close now.

lib/Headers/altivec.h

 static __inline__ vector unsigned long long __attribute__((__always_inline__))
 vec_bperm(vector unsigned __int128 __a, vector unsigned char __b) {
   return __builtin_altivec_vbpermq((vector unsigned char)__a,
                                    (vector unsigned char)__b);
 }
 #endif
 #endif
 
+
+/* vec_reve */
+
+static inline __ATTRS_o_ai vector bool char vec_reve(vector bool char __a) {
+  vector bool char __vec = __builtin_shufflevector(
+      __a, __a, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector signed char vec_reve(vector signed char __a) {
+  vector signed char __vec = __builtin_shufflevector(
+      __a, __a, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector unsigned char
+vec_reve(vector unsigned char __a) {
+  vector unsigned char __vec = __builtin_shufflevector(
+      __a, __a, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector bool int vec_reve(vector bool int __a) {
+  vector bool int __vec = __builtin_shufflevector(__a, __a, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector signed int vec_reve(vector signed int __a) {
+  vector signed int __vec = __builtin_shufflevector(__a, __a, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector unsigned int
+vec_reve(vector unsigned int __a) {
+  vector unsigned int __vec = __builtin_shufflevector(__a, __a, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector bool short vec_reve(vector bool short __a) {
+  vector bool short __vec =
+      __builtin_shufflevector(__a, __a, 7, 6, 5, 4, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector signed short
+vec_reve(vector signed short __a) {
+  vector signed short __vec =
+      __builtin_shufflevector(__a, __a, 7, 6, 5, 4, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector unsigned short
+vec_reve(vector unsigned short __a) {
+  vector unsigned short __vec =
+      __builtin_shufflevector(__a, __a, 7, 6, 5, 4, 3, 2, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector float vec_reve(vector float __a) {
+  vector float __vec = __builtin_shufflevector(__a, __a, 3, 2, 1, 0);
+  return __vec;
+}
+
+#ifdef __VSX__
+static inline __ATTRS_o_ai vector bool long long
+vec_reve(vector bool long long __a) {
+  vector bool long long __vec = __builtin_shufflevector(__a, __a, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector signed long long
+vec_reve(vector signed long long __a) {
+  vector signed long long __vec = __builtin_shufflevector(__a, __a, 1, 0);
+  return __vec;
+}
+
+static inline __ATTRS_o_ai vector unsigned long long
+vec_reve(vector unsigned long long __a) {
+  vector unsigned long long __vec = __builtin_shufflevector(__a, __a, 1, 0);
+  return __vec;
+}
+static inline __ATTRS_o_ai vector double vec_reve(vector double __a) {
+  vector double __vec = __builtin_shufflevector(__a, __a, 1, 0);
+  return __vec;
+}
+#endif
+
 #undef __ATTRS_o_ai
 
 #endif /* __ALTIVEC_H */

test/CodeGen/builtins-ppc-altivec.c

 vector short vs = { -1, 2, -3, 4, -5, 6, -7, 8 };
 vector unsigned short vus = { 1, 2, 3, 4, 5, 6, 7, 8 };
 vector pixel vp = { 1, 2, 3, 4, 5, 6, 7, 8 };
 vector bool int vbi = { 1, 0, 1, 0 };
 vector int vi = { -1, 2, -3, 4 };
 vector unsigned int vui = { 1, 2, 3, 4 };
 vector float vf = { -1.5, 2.5, -3.5, 4.5 };
 
 vector bool char res_vbc;
 vector signed char res_vsc;
 vector unsigned char res_vuc;
 vector bool short res_vbs;
 vector short res_vs;
 vector unsigned short res_vus;
 vector pixel res_vp;
 vector bool int res_vbi;
 vector int res_vi;
// CHECK-LE: @llvm.ppc.altivec.vcmpgtfp.p(i32 2
   res_i = (vf1 <= vf2);
 // CHECK: @llvm.ppc.altivec.vcmpgefp.p(i32 2
 // CHECK-LE: @llvm.ppc.altivec.vcmpgefp.p(i32 2
 
   res_i = (vf1 >= vf2);
 // CHECK: @llvm.ppc.altivec.vcmpgefp.p(i32 2
 // CHECK-LE: @llvm.ppc.altivec.vcmpgefp.p(i32 2
 }
+
+// CHECK-LABEL: define void @test8
+// CHECK-LE-LABEL: define void @test8
+void test8() {
+  res_vbc = vec_reve(vbc);
+  // CHECK: shufflevector <16 x i8> %{{[0-9]+}}, <16 x i8> %{{[0-9]+}}, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <16 x i8> %{{[0-9]+}}, <16 x i8> %{{[0-9]+}}, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+
+  res_vsc = vec_reve(vsc);
+  // CHECK: shufflevector <16 x i8> %{{[0-9]+}}, <16 x i8> %{{[0-9]+}}, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <16 x i8> %{{[0-9]+}}, <16 x i8> %{{[0-9]+}}, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+
+  res_vuc = vec_reve(vuc);
+  // CHECK: shufflevector <16 x i8> %{{[0-9]+}}, <16 x i8> %{{[0-9]+}}, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <16 x i8> %{{[0-9]+}}, <16 x i8> %{{[0-9]+}}, <16 x i32> <i32 15, i32 14, i32 13, i32 12, i32 11, i32 10, i32 9, i32 8, i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+
+  res_vbi = vec_reve(vbi);
+  // CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+
+  res_vi = vec_reve(vi);
+  // CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+
+  res_vui = vec_reve(vui);
+  // CHECK: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <4 x i32> %{{[0-9]+}}, <4 x i32> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+
+  res_vbs = vec_reve(vbs);
+  // CHECK: shufflevector <8 x i16> %{{[0-9]+}}, <8 x i16> %{{[0-9]+}}, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <8 x i16> %{{[0-9]+}}, <8 x i16> %{{[0-9]+}}, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+
+  res_vbs = vec_reve(vs);
+  // CHECK: shufflevector <8 x i16> %{{[0-9]+}}, <8 x i16> %{{[0-9]+}}, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <8 x i16> %{{[0-9]+}}, <8 x i16> %{{[0-9]+}}, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+
+  res_vbs = vec_reve(vus);
+  // CHECK: shufflevector <8 x i16> %{{[0-9]+}}, <8 x i16> %{{[0-9]+}}, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <8 x i16> %{{[0-9]+}}, <8 x i16> %{{[0-9]+}}, <8 x i32> <i32 7, i32 6, i32 5, i32 4, i32 3, i32 2, i32 1, i32 0>
+
+  res_vf = vec_reve(vf);
+  // CHECK: shufflevector <4 x float> %{{[0-9]+}}, <4 x float> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+  // CHECK-LE: shufflevector <4 x float> %{{[0-9]+}}, <4 x float> %{{[0-9]+}}, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
+
+}

test/CodeGen/builtins-ppc-vsx.c

 // CHECK-LE: uitofp <2 x i64> %{{.*}} to <2 x double>
 // CHECK-LE: fmul <2 x double>
 
   res_vd = vec_ctf(vull, 31);
 // CHECK: uitofp <2 x i64> %{{.*}} to <2 x double>
 // CHECK: fmul <2 x double>
 // CHECK-LE: uitofp <2 x i64> %{{.*}} to <2 x double>
 // CHECK-LE: fmul <2 x double>
+
+  res_vbll = vec_reve(vbll);
+  // CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+  // CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+
+  res_vsll = vec_reve(vsll);
+  // CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+  // CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+
+  res_vull = vec_reve(vull);
+  // CHECK: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+  // CHECK-LE: shufflevector <2 x i64> %{{[0-9]+}}, <2 x i64> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+
+  res_vd = vec_reve(vd);
+  // CHECK: shufflevector <2 x double> %{{[0-9]+}}, <2 x double> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
+  // CHECK-LE: shufflevector <2 x double> %{{[0-9]+}}, <2 x double> %{{[0-9]+}}, <2 x i32> <i32 1, i32 0>
 }