Author: chandlerc Date: Wed Sep 30 21:21:34 2015 New Revision: 248980 URL: http://llvm.org/viewvc/llvm-project?rev=248980&view=rev Log: Patch over a really horrible bug in our vector builtins that showed up recently when we started using direct conversion to model sign extension. The __v16qi type we use for SSE v16i8 vectors is defined in terms of 'char' which may or may not be signed! This causes us to generate pmovsx and pmovzx depending on the setting of -funsigned-char.
This patch just forms an explicitly signed type and uses that to formulate the sign extension. While this gets the correct behavior (which we now verify with the enhanced test) this is just the tip of the ice berg. Now that I know what to look for, I have found errors of this sort *throughout* our vector code. Fortunately, this is the only specific place where I know of users actively having their code miscompiled by Clang due to this, so I'm keeping the fix for those users minimal and targeted. I'll be sending a proper email for discussion of how to fix these systematically, what the implications are, and just how widely broken this is... From what I can tell, we have never shipped a correct set of builtin headers for x86 when users rely on -funsigned-char. Oops. Modified: cfe/trunk/lib/Headers/smmintrin.h cfe/trunk/test/CodeGen/sse41-builtins.c Modified: cfe/trunk/lib/Headers/smmintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/smmintrin.h?rev=248980&r1=248979&r2=248980&view=diff ============================================================================== --- cfe/trunk/lib/Headers/smmintrin.h (original) +++ cfe/trunk/lib/Headers/smmintrin.h Wed Sep 30 21:21:34 2015 @@ -286,19 +286,34 @@ _mm_cmpeq_epi64(__m128i __V1, __m128i __ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cvtepi8_epi16(__m128i __V) { - return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, (__v16qi)__V, 0, 1, 2, 3, 4, 5, 6, 7), __v8hi); + /* We need a local definitively signed typedef similar to __v16qi even in the + * presence of __UNSIGNED_CHAR__. + * FIXME: __v16qi should almost certainly be definitively signed. + */ + typedef signed char __signed_v16qi __attribute__((__vector_size__(16))); + return (__m128i)__builtin_convertvector(__builtin_shufflevector((__signed_v16qi)__V, (__signed_v16qi)__V, 0, 1, 2, 3, 4, 5, 6, 7), __v8hi); } static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cvtepi8_epi32(__m128i __V) { - return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, (__v16qi)__V, 0, 1, 2, 3), __v4si); + /* We need a local definitively signed typedef similar to __v16qi even in the + * presence of __UNSIGNED_CHAR__. + * FIXME: __v16qi should almost certainly be definitively signed. + */ + typedef signed char __signed_v16qi __attribute__((__vector_size__(16))); + return (__m128i)__builtin_convertvector(__builtin_shufflevector((__signed_v16qi)__V, (__signed_v16qi)__V, 0, 1, 2, 3), __v4si); } static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cvtepi8_epi64(__m128i __V) { - return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qi)__V, (__v16qi)__V, 0, 1), __v2di); + /* We need a local definitively signed typedef similar to __v16qi even in the + * presence of __UNSIGNED_CHAR__. + * FIXME: __v16qi should almost certainly be definitively signed. + */ + typedef signed char __signed_v16qi __attribute__((__vector_size__(16))); + return (__m128i)__builtin_convertvector(__builtin_shufflevector((__signed_v16qi)__V, (__signed_v16qi)__V, 0, 1), __v2di); } static __inline__ __m128i __DEFAULT_FN_ATTRS Modified: cfe/trunk/test/CodeGen/sse41-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sse41-builtins.c?rev=248980&r1=248979&r2=248980&view=diff ============================================================================== --- cfe/trunk/test/CodeGen/sse41-builtins.c (original) +++ cfe/trunk/test/CodeGen/sse41-builtins.c Wed Sep 30 21:21:34 2015 @@ -1,6 +1,8 @@ // REQUIRES: x86-registered-target // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.1 -emit-llvm -o - -Werror | FileCheck %s +// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.1 -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.1 -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM +// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.1 -fno-signed-char -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM // Don't include mm_malloc.h, it's system specific. #define __MM_MALLOC_H _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits