nemanjai added inline comments.
================
Comment at: lib/Headers/altivec.h:136
@@ -131,3 +135,3 @@
#if defined(__POWER8_VECTOR__) && defined(__powerpc64__)
static vector double __ATTRS_o_ai vec_abs(vector double __a) {
----------------
I thought we were going to change the guard here to __VSX__ rather than
__POWER8_VECTOR.
================
Comment at: test/CodeGen/builtins-ppc-altivec.c:3
@@ +2,3 @@
+// 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 \
----------------
Just a minor nit. Please indent the continuation lines for readability.
================
Comment at: test/CodeGen/builtins-ppc-altivec.c:8
@@ +7,3 @@
+// RUN: -o - | FileCheck %s -check-prefix=CHECK-LE
+// RUN: not %clang_cc1 -triple powerpc64le-unknown-unknown -emit-llvm %s \
+// RUN: -o - 2>&1 | FileCheck %s -check-prefix=CHECK-NOALTIVEC
----------------
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.
================
Comment at: test/CodeGen/builtins-ppc-altivec.c:84
@@ +83,3 @@
+// CHECK-LE: store <4 x float> %{{.*}}, <4 x float>* @vf
+// CHECK-NOALTIVEC: error: use of undeclared identifier 'vf'
+// CHECK-NOALTIVEC: vf = vec_abs(vf)
----------------
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).
================
Comment at: test/CodeGen/builtins-ppc-p8vector.c:76
@@ -75,5 +75,3 @@
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> %{{[0-9]*}})
+// CHECK: store <2 x double> %{{.*}}, <2 x double>* @res_vd
----------------
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.
http://reviews.llvm.org/D17816
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
