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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@248980 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Headers/smmintrin.h b/lib/Headers/smmintrin.h
index 4745b1c5831ac829e54162024df4ff80f9a2a78b..7a18201ecedf406fbebe04a37d072a7a186efad1 100644 (file)
--- a/lib/Headers/smmintrin.h
+++ b/lib/Headers/smmintrin.h
@@ -286,19 +286,34 @@ _mm_cmpeq_epi64(__m128i __V1, __m128i __V2)
 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

diff --git a/test/CodeGen/sse41-builtins.c b/test/CodeGen/sse41-builtins.c
index b3ba22d3c800e3a887efc2cf39c46f7084d68129..486c8a30a9236f0c56577ba1a2018bb9c5400459 100644 (file)
--- a/test/CodeGen/sse41-builtins.c
+++ b/test/CodeGen/sse41-builtins.c
@@ -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