[PPC] FE support for generating VSX [negated] absolute value instructions

[amehsan]

Summary

Includes new built-in, conversion of built-in to target-independent intrinsic and update in the header file. Tests are also updated. There is a second part in the backend for which I will post a separate code-review. BACKEND PART SHOULD BE COMMITTED FIRST.

[nemanjai]

lib/Headers/altivec.h

136

I thought we were going to change the guard here to VSX rather than __POWER8_VECTOR.

test/CodeGen/builtins-ppc-altivec.c

3

Just a minor nit. Please indent the continuation lines for readability.

8

I would have thought that we already have a diagnostic test case for the missing -faltivec, but if we don't, thanks for adding it.

85

I am not recommending you change anything here, just want to point out that there's a potential concern with adding diagnostic test cases in with functional ones (and yes, I know we have lots of this already). Namely, the concern is that the diagnostic may go away as we add more code to the test case and the particular error you're looking for goes past the error threshold (I think the default is 20 errors).

test/CodeGen/builtins-ppc-p8vector.c

76–79

I think that without asserts, clang sometimes gives temporary variables names rather than numbers. I'd recommend getting rid of the regex for the argument to @llvm.fabs.* so that you don't get into a weird situation where some build bot somewhere fails due to this change.

[amehsan]

lib/Headers/altivec.h

136

I want to go through the file separately, check for similar inaccuracies, and fix everything of that kind in one separate commit. This may also require moving test cases between different files. So I do not want to mix it with another submission.

test/CodeGen/builtins-ppc-altivec.c

8

No we don't. I spent some time looking for it in the existing test cases and did not find anything.

85

Would it address your concern if I add "-ferror-limit 0" to the command line?

test/CodeGen/builtins-ppc-p8vector.c

76–79

That pattern is used in other tests. (You can see that in the one right above this one.) But I am fine with changing it to a more general regex like {{.*}}. (I can also remove it but I really wanted to close the parenthesis :)

[nemanjai]

All of my comments are just nits and shouldn't hold up approval. As far as I can tell this looks fine, but I'll let the LGTM come from Kit or Hal.

lib/Headers/altivec.h

136

That would be great! Thank you.

test/CodeGen/builtins-ppc-altivec.c

85

I think that's a great idea and good general guidance for test cases of this type. BTW, no need to post another review for this - just change it in the commit once this patch is approved.

test/CodeGen/builtins-ppc-p8vector.c

76–79

I see what you mean, it's in the previous CHECK pattern. You should be safe keeping this in. However, I'd recommend that you try a build without asserts (especially for FE changes) just to make sure things don't behave differently. The only reason I bring it up is because I've been burned by that before :).

[amehsan]

added -ferror-limit 0 to the command line for no-altivec test.
fixed indentation of RUN commands
confirmed that all tests pass in a release and minrelsize build types
changed {{[0-9]*}} to {{.*}} (it was fine but no need to put a restrictions in the tests that we do not care about).

[kbarton]

LGTM

[kbarton]

Committed r263051

include/clang/Basic/BuiltinsPPC.def

 BUILTIN(__builtin_vsx_xvsqrtdp, "V2dV2d", "")
 BUILTIN(__builtin_vsx_xvsqrtsp, "V4fV4f", "")
 
 BUILTIN(__builtin_vsx_xxleqv, "V4UiV4UiV4Ui", "")
 
 BUILTIN(__builtin_vsx_xvcpsgndp, "V2dV2dV2d", "")
 BUILTIN(__builtin_vsx_xvcpsgnsp, "V4fV4fV4f", "")
 
+BUILTIN(__builtin_vsx_xvabssp, "V4fV4f", "")
+BUILTIN(__builtin_vsx_xvabsdp, "V2dV2d", "")
+
 // HTM builtins
 BUILTIN(__builtin_tbegin, "UiUIi", "")
 BUILTIN(__builtin_tend, "UiUIi", "")
 
 BUILTIN(__builtin_tabort, "UiUi", "")
 BUILTIN(__builtin_tabortdc, "UiUiUiUi", "")
 BUILTIN(__builtin_tabortdci, "UiUiUii", "")
 BUILTIN(__builtin_tabortwc, "UiUiUiUi", "")

lib/CodeGen/CGBuiltin.cpp

else if (BuiltinID == PPC::BI__builtin_vsx_xvrdpip ||
              BuiltinID == PPC::BI__builtin_vsx_xvrspip)
       ID = Intrinsic::ceil;
     else if (BuiltinID == PPC::BI__builtin_vsx_xvrdpiz ||
              BuiltinID == PPC::BI__builtin_vsx_xvrspiz)
       ID = Intrinsic::trunc;
     llvm::Function *F = CGM.getIntrinsic(ID, ResultType);
     return Builder.CreateCall(F, X);
   }
+
+  // Absolute value
+  case PPC::BI__builtin_vsx_xvabsdp:
+  case PPC::BI__builtin_vsx_xvabssp: {
+    llvm::Type *ResultType = ConvertType(E->getType());
+    Value *X = EmitScalarExpr(E->getArg(0));
+    llvm::Function *F = CGM.getIntrinsic(Intrinsic::fabs, ResultType);
+    return Builder.CreateCall(F, X);
+  }
+
   // FMA variations
   case PPC::BI__builtin_vsx_xvmaddadp:
   case PPC::BI__builtin_vsx_xvmaddasp:
   case PPC::BI__builtin_vsx_xvnmaddadp:
   case PPC::BI__builtin_vsx_xvnmaddasp:
   case PPC::BI__builtin_vsx_xvmsubadp:
   case PPC::BI__builtin_vsx_xvmsubasp:
   case PPC::BI__builtin_vsx_xvnmsubadp:

lib/Headers/altivec.h

 #if defined(__POWER8_VECTOR__) && defined(__powerpc64__)
 static vector signed long long __ATTRS_o_ai
 vec_abs(vector signed long long __a) {
   return __builtin_altivec_vmaxsd(__a, -__a);
 }
 #endif
 
 static vector float __ATTRS_o_ai vec_abs(vector float __a) {
+#ifdef __VSX__
+  return __builtin_vsx_xvabssp(__a);
+#else
   vector unsigned int __res =
       (vector unsigned int)__a & (vector unsigned int)(0x7FFFFFFF);
   return (vector float)__res;
+#endif
 }
 
 #if defined(__POWER8_VECTOR__) && defined(__powerpc64__)
 static vector double __ATTRS_o_ai vec_abs(vector double __a) {
-  vector unsigned long long __res = { 0x7FFFFFFFFFFFFFFF, 0x7FFFFFFFFFFFFFFF };
-  __res &= (vector unsigned int)__a;
-  return (vector double)__res;
+  return __builtin_vsx_xvabsdp(__a);
 }
 #endif
 
 /* vec_abss */
 #define __builtin_altivec_abss_v16qi vec_abss
 #define __builtin_altivec_abss_v8hi vec_abss
 #define __builtin_altivec_abss_v4si vec_abss
 

test/CodeGen/builtins-ppc-altivec.c

 // REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -faltivec -triple powerpc-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -faltivec -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s -check-prefix=CHECK-LE
+// RUN: %clang_cc1 -faltivec -triple powerpc-unknown-unknown -emit-llvm %s \
+// RUN:            -o - | FileCheck %s
+// RUN: %clang_cc1 -faltivec -triple powerpc64-unknown-unknown -emit-llvm %s \
+// RUN:            -o - | FileCheck %s
+// RUN: %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -emit-llvm %s \
+// RUN:            -o - | FileCheck %s -check-prefix=CHECK-LE
+// RUN: not %clang_cc1 -triple powerpc64le-unknown-unknown -emit-llvm %s \
+// RUN:            -ferror-limit 0 -o - 2>&1 \
+// RUN:            | FileCheck %s -check-prefix=CHECK-NOALTIVEC
 
 vector bool char vbc = { 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0 };
 vector signed char vsc = { 1, -2, 3, -4, 5, -6, 7, -8, 9, -10, 11, -12, 13, -14, 15, -16 };
 vector unsigned char vuc = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
 vector bool short vbs = { 1, 0, 1, 0, 1, 0, 1, 0 };
 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 short res_vs;
 vector unsigned short res_vus;
 vector pixel res_vp;
 vector bool int res_vbi;
 vector int res_vi;
 vector unsigned int res_vui;
 vector float res_vf;
 
+// CHECK-NOALTIVEC: error: unknown type name 'vector'
+
 signed char param_sc;
 unsigned char param_uc;
 short param_s;
 unsigned short param_us;
 int param_i;
 unsigned int param_ui;
 float param_f;
 
// CHECK-LE: @llvm.ppc.altivec.vmaxsh
 
   vi = vec_abs(vi);
 // CHECK: sub <4 x i32> zeroinitializer
 // CHECK: @llvm.ppc.altivec.vmaxsw
 // CHECK-LE: sub <4 x i32> zeroinitializer
 // CHECK-LE: @llvm.ppc.altivec.vmaxsw
 
   vf = vec_abs(vf);
-// CHECK: and <4 x i32>
-// CHECK-LE: and <4 x i32>
+// CHECK: bitcast <4 x float> %{{.*}} to <4 x i32>
+// CHECK: and <4 x i32> {{.*}}, <i32 2147483647, i32 2147483647, i32 2147483647, i32 2147483647>
+// CHECK: bitcast <4 x i32> %{{.*}} to <4 x float>
+// CHECK: store <4 x float> %{{.*}}, <4 x float>* @vf
+// CHECK-LE: bitcast <4 x float> %{{.*}} to <4 x i32>
+// CHECK-LE: and <4 x i32> {{.*}}, <i32 2147483647, i32 2147483647, i32 2147483647, i32 2147483647>
+// CHECK-LE: bitcast <4 x i32> %{{.*}} to <4 x float>
+// CHECK-LE: store <4 x float> %{{.*}}, <4 x float>* @vf
+// CHECK-NOALTIVEC: error: use of undeclared identifier 'vf'
+// CHECK-NOALTIVEC: vf = vec_abs(vf) 
 
   /* vec_abs */
   vsc = vec_abss(vsc);
 // CHECK: @llvm.ppc.altivec.vsubsbs
 // CHECK: @llvm.ppc.altivec.vmaxsb
 // CHECK-LE: @llvm.ppc.altivec.vsubsbs
 // CHECK-LE: @llvm.ppc.altivec.vmaxsb
 

test/CodeGen/builtins-ppc-p8vector.c

void test1() {
 
   /* vec_abs */
   res_vsll = vec_abs(vsll);
 // CHECK: call <2 x i64> @llvm.ppc.altivec.vmaxsd(<2 x i64> %{{[0-9]*}}, <2 x i64>
 // CHECK-LE: call <2 x i64> @llvm.ppc.altivec.vmaxsd(<2 x i64> %{{[0-9]*}}, <2 x i64>
 // CHECK-PPC: error: call to 'vec_abs' is ambiguous
 
   res_vd = vec_abs(vda);
-// CHECK: store <2 x i64> <i64 9223372036854775807, i64 9223372036854775807>, <2 x i64>*
-// CHECK: and <2 x i64>
-// CHECK-LE: store <2 x i64> <i64 9223372036854775807, i64 9223372036854775807>, <2 x i64>*
-// CHECK-LE: and <2 x i64>
+// CHECK: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
+// CHECK: store <2 x double> %{{.*}}, <2 x double>* @res_vd
+// CHECK-LE: call <2 x double> @llvm.fabs.v2f64(<2 x double> %{{.*}})
+// CHECK-LE: store <2 x double> %{{.*}}, <2 x double>* @res_vd
 // CHECK-PPC: error: call to 'vec_abs' is ambiguous
 
   /* vec_add */
   res_vsll = vec_add(vsll, vsll);
 // CHECK: add <2 x i64>
 // CHECK-LE: add <2 x i64>
 // CHECK-PPC: error: call to 'vec_add' is ambiguous
 

test/CodeGen/builtins-ppc-vsx.c

 double res_d;
 
 void dummy() { }
 
 void test1() {
 // CHECK-LABEL: define void @test1
 // CHECK-LE-LABEL: define void @test1
 
+  res_vf = vec_abs(vf);
+// CHECK: call <4 x float> @llvm.fabs.v4f32(<4 x float> %{{[0-9]*}})
+// CHECK-LE: call <4 x float> @llvm.fabs.v4f32(<4 x float> %{{[0-9]*}})
+
+  dummy();
+// CHECK: call void @dummy()
+// CHECK-LE: call void @dummy()
+
   res_vd = vec_add(vd, vd);
 // CHECK: fadd <2 x double>
 // CHECK-LE: fadd <2 x double>
 
   res_vd = vec_and(vbll, vd);
 // CHECK: and <2 x i64>
 // CHECK: bitcast <2 x i64> %{{[0-9]*}} to <2 x double>
 // CHECK-LE: and <2 x i64>