[PATCH] D27251: [PPC] some bugs mainly about sign problem fixed in altivec.h

Tony Jiang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 30 09:04:08 PST 2016


jtony added inline comments.


================
Comment at: lib/Headers/altivec.h:16456
 
 #ifdef __VSX__
 static __inline__ vector signed long long __ATTRS_o_ai
----------------
Thanks  a lot for your good catch for the macro issue in vec_xst_be, that's a good catch. BTW, Can you move this up also like vec_xst_be?


================
Comment at: test/CodeGen/builtins-ppc-vsx.c:1696
+
+signed char param_sc;
+unsigned char param_uc;
----------------
I would prefer these definitions occur at the beginning of the file like before. 


================
Comment at: test/CodeGen/builtins-ppc-vsx.c:1706
+/* ----------------------------- vec_xl_be ---------------------------------- */
+void test2() {
+  // CHECK-LABEL: define void @test2
----------------
These test cases should be grouped together with the test  cases from 1663 - 1683. Put the vec_xl_be overloads together,  and the vec_xst_be together (maybe after vec_xl_be). I am OK with either put these test2 and test3 into test 1, or make them stand-alone, as long as these overloaded test cases for vec_xst_be and vec_xl_be are put together seperately. Thanks for you good catch, this problem is not found in our previous code review.


https://reviews.llvm.org/D27251





More information about the cfe-commits mailing list