nemanjai added a comment.
Zaara, prior to committing this patch, can you write a test case that will
actually use these builtins in an executable test case and confirm the results.
We'll then run it on the simulator to ensure functional correctness.
================
Comment at: lib/Headers/altivec.h:38
#define __ATTRS_o_ai __attribute__((__overloadable__, __always_inline__))
+#include <stddef.h>
----------------
I think size_t is only used for the load/store with length which are Power9
specific. I think we should guard this include with that macro as well to avoid
changing behaviour of existing code inadvertently.
================
Comment at: lib/Headers/altivec.h:2609
+vec_xl_len(signed char * __a, size_t __b) {
+ return (vector signed char)__builtin_vsx_lxvl(__a, (__b << 56));
+}
----------------
This is undefined behaviour in 32-bit mode. Please guard for 64-bit mode unless
this is already in such a block.
================
Comment at: test/CodeGen/builtins-ppc-p9vector.c:29
+float af[4] = {23.4f, 56.7f, 89.0f, 12.3f};
+double ad[2] = {23.4, 56.7};
----------------
I don't think we need to initialize these. And if we keep the initialization,
let's use the same formatting (i.e. left curly brace, space, list of
initializers, space, right curly brace).
================
Comment at: test/CodeGen/builtins-ppc-p9vector.c:35
+ 0, 1, 2, 3, 4, 5, 6, 7};
+signed short ass[8] = { -1, 2, -3, 4, -5, 6, -7, 8 };
+unsigned short aus[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
----------------
Please select a naming scheme that does not lead to such unfortunate variable
names.
https://reviews.llvm.org/D26304
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
