[PATCH] D26160: Implement remaining vector comparison builtins

Nemanja Ivanovic via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 00:37:39 PDT 2016


nemanjai added a comment.

I think that other than the few minor nits, this patch is fine.



================
Comment at: lib/Headers/altivec.h:1556
 static __inline__ vector bool char __ATTRS_o_ai
+vec_cmpeq(vector bool char __a, vector bool char __b) {
+  return (vector bool char)__builtin_altivec_vcmpequb((vector char)__a,
----------------
When we have all three overloads, the order in the rest of the file is typically:
1. signed <TYPE>
2. unsigned <TYPE>
3. bool <TYPE>
Let's keep this consistent please.


================
Comment at: lib/Headers/altivec.h:2379
 
-static __inline__ unsigned __ATTRS_o_ai
-vec_first_match_or_eos_index(vector unsigned short __a,
-                             vector unsigned short __b) {
+static __inline__ unsigned __ATTRS_o_ai vec_first_match_or_eos_index(
+    vector unsigned short __a, vector unsigned short __b) {
----------------
I assume that this change was made by clang-format. However, I think this is one of those examples where not formatting according to its rules makes the code more readable. So I personally prefer the old formatting.


https://reviews.llvm.org/D26160





More information about the llvm-commits mailing list