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