Fix the SSE4 byte sign extension in a cleaner way, and more thoroughly test that our intrinsics behave the same under -fsigned-char and -funsigned-char.

[chandlerc]

Summary

This further testing uncovered that AVX-2 has a broken cmpgt for 8-bit
elements, and has for a long time. This is fixed in the same way as
SSE4 handles the case.

The other ISA extensions currently work correctly because they use
specific instruction intrinsics. As soon as they are rewritten in terms
of generic IR, they will need to add these special casts. I've added the
necessary testing to catch this however, so we shouldn't have to chase
it down again.

I considered changing the core typedef to be signed, but that seems like
a bad idea. Notably, it would be an ABI break if anyone is reaching into
the innards of the intrinsic headers and passing __v16qi on an API
boundary. I can't be completely confident that this wouldn't happen due
to a macro expanding in a lambda, etc., so it seems much better to leave
it alone. It also matches GCC's behavior exactly.

A fun side note is that for both GCC and Clang, -funsigned-char really
does change the semantics of __v16qi. To observe this, consider:

% cat x.cc
#include <smmintrin.h>
#include <iostream>

int main() {
  __v16qi a = { 1, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
  __v16qi b = _mm_set1_epi8(-1);
  std::cout << (int)(a / b)[0] << ", " << (int)(a / b)[1] << '\n';
}
% clang++ -o x x.cc && ./x
-1, 1
% clang++ -funsigned-char -o x x.cc && ./x
0, 1

However, while this may be surprising, both Clang and GCC agree.

[rsmith]

LGTM

cfe/trunk/lib/Headers/avx2intrin.h

 _mm256_cmpeq_epi64(__m256i __a, __m256i __b)
 {
   return (__m256i)(__a == __b);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS
 _mm256_cmpgt_epi8(__m256i __a, __m256i __b)
 {
-  return (__m256i)((__v32qi)__a > (__v32qi)__b);
+  /* This function always performs a signed comparison, but __v32qi is a char
+     which may be signed or unsigned, so use __v32qs. */
+  return (__m256i)((__v32qs)__a > (__v32qs)__b);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS
 _mm256_cmpgt_epi16(__m256i __a, __m256i __b)
 {
   return (__m256i)((__v16hi)__a > (__v16hi)__b);
 }
 

cfe/trunk/lib/Headers/avxintrin.h

 
 typedef double __v4df __attribute__ ((__vector_size__ (32)));
 typedef float __v8sf __attribute__ ((__vector_size__ (32)));
 typedef long long __v4di __attribute__ ((__vector_size__ (32)));
 typedef int __v8si __attribute__ ((__vector_size__ (32)));
 typedef short __v16hi __attribute__ ((__vector_size__ (32)));
 typedef char __v32qi __attribute__ ((__vector_size__ (32)));
 
+/* We need an explicitly signed variant for char. Note that this shouldn't
+ * appear in the interface though. */
+typedef signed char __v32qs __attribute__((__vector_size__(32)));
+
 typedef float __m256 __attribute__ ((__vector_size__ (32)));
 typedef double __m256d __attribute__((__vector_size__(32)));
 typedef long long __m256i __attribute__((__vector_size__(32)));
 
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("avx")))
 
 /* Arithmetic */

cfe/trunk/lib/Headers/emmintrin.h

 typedef long long __m128i __attribute__((__vector_size__(16)));
 
 /* Type defines.  */
 typedef double __v2df __attribute__ ((__vector_size__ (16)));
 typedef long long __v2di __attribute__ ((__vector_size__ (16)));
 typedef short __v8hi __attribute__((__vector_size__(16)));
 typedef char __v16qi __attribute__((__vector_size__(16)));
 
+/* We need an explicitly signed variant for char. Note that this shouldn't
+ * appear in the interface though. */
+typedef signed char __v16qs __attribute__((__vector_size__(16)));
+
 #include <f16cintrin.h>
 
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("sse2")))
 
 static __inline__ __m128d __DEFAULT_FN_ATTRS
 _mm_add_sd(__m128d __a, __m128d __b)
 {

 {
   return (__m128i)((__v4si)__a == (__v4si)__b);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cmpgt_epi8(__m128i __a, __m128i __b)
 {
   /* This function always performs a signed comparison, but __v16qi is a char
-     which may be signed or unsigned. */
-  typedef signed char __v16qs __attribute__((__vector_size__(16)));
+     which may be signed or unsigned, so use __v16qs. */
   return (__m128i)((__v16qs)__a > (__v16qs)__b);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cmpgt_epi16(__m128i __a, __m128i __b)
 {
   return (__m128i)((__v8hi)__a > (__v8hi)__b);
 }

cfe/trunk/lib/Headers/smmintrin.h

 {
   return (__m128i)((__v2di)__V1 == (__v2di)__V2);
 }
 
 /* SSE4 Packed Integer Sign-Extension.  */
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi16(__m128i __V)
 {
-  /* 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);
+  /* This function always performs a signed extension, but __v16qi is a char
+     which may be signed or unsigned, so use __v16qs. */
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qs)__V, (__v16qs)__V, 0, 1, 2, 3, 4, 5, 6, 7), __v8hi);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi32(__m128i __V)
 {
-  /* 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);
+  /* This function always performs a signed extension, but __v16qi is a char
+     which may be signed or unsigned, so use __v16qs. */
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qs)__V, (__v16qs)__V, 0, 1, 2, 3), __v4si);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi8_epi64(__m128i __V)
 {
-  /* 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);
+  /* This function always performs a signed extension, but __v16qi is a char
+     which may be signed or unsigned, so use __v16qs. */
+  typedef signed char __v16qs __attribute__((__vector_size__(16)));
+  return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v16qs)__V, (__v16qs)__V, 0, 1), __v2di);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_cvtepi16_epi32(__m128i __V)
 {
   return (__m128i)__builtin_convertvector(__builtin_shufflevector((__v8hi)__V, (__v8hi)__V, 0, 1, 2, 3), __v4si);
 }
 

cfe/trunk/test/CodeGen/avx2-builtins.c

 // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +avx2 -emit-llvm -o - -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +avx2 -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s
 // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +avx2 -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +avx2 -fno-signed-char -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM
 
 // REQUIRES: x86-registered-target
 
 // Don't include mm_malloc.h, it's system specific.
 #define __MM_MALLOC_H
 
 #include <immintrin.h>
 

cfe/trunk/test/CodeGen/avx512bw-builtins.c

 // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -ffreestanding -target-feature +avx512bw -emit-llvm -o - -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -ffreestanding -target-feature +avx512bw -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s
 
 #include <immintrin.h>
 
 __mmask64 test_mm512_cmpeq_epi8_mask(__m512i __a, __m512i __b) {
   // CHECK-LABEL: @test_mm512_cmpeq_epi8_mask
   // CHECK: @llvm.x86.avx512.mask.pcmpeq.b.512
   return (__mmask64)_mm512_cmpeq_epi8_mask(__a, __b);
 }

cfe/trunk/test/CodeGen/avx512vlbw-builtins.c

 // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -ffreestanding -target-feature +avx512bw -target-feature +avx512vl -emit-llvm -o - -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -ffreestanding -target-feature +avx512bw -target-feature +avx512vl -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s
 
 #include <immintrin.h>
 
 __mmask32 test_mm256_cmpeq_epi8_mask(__m256i __a, __m256i __b) {
   // CHECK-LABEL: @test_mm256_cmpeq_epi8_mask
   // CHECK: @llvm.x86.avx512.mask.pcmpeq.b.256
   return (__mmask32)_mm256_cmpeq_epi8_mask(__a, __b);
 }

cfe/trunk/test/CodeGen/mmx-builtins.c

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 %s -O3 -triple=x86_64-apple-darwin -target-feature +ssse3 -S -o - | FileCheck %s
+// RUN: %clang_cc1 %s -O3 -triple=x86_64-apple-darwin -target-feature +ssse3 -fno-signed-char -S -o - | FileCheck %s
 
 // FIXME: Disable inclusion of mm_malloc.h, our current implementation is broken
 // on win32 since we don't generally know how to find errno.h.
 #define __MM_MALLOC_H
 
 #include <tmmintrin.h>
 
 __m64 test1(__m64 a, __m64 b) {

cfe/trunk/test/CodeGen/sse42-builtins.c

 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.2 -fno-signed-char -emit-llvm -o - -Werror | FileCheck %s
 // RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.2 -S -o - -Werror | FileCheck %s --check-prefix=CHECK-ASM
+// RUN: %clang_cc1 %s -O0 -triple=x86_64-apple-darwin -target-feature +sse4.2 -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
 
 #include <x86intrin.h>
 
+__m128i test_mm_cmpgt_epi8(__m128i A, __m128i B) {
+  // CHECK-LABEL: test_mm_cmpgt_epi8
+  // CHECK: icmp sgt <16 x i8>
+  // CHECK-ASM: pcmpgtb %xmm{{.*}}, %xmm{{.*}}
+  return _mm_cmpgt_epi8(A, B);
+}
+
+__m128i test_mm_cmpgt_epi16(__m128i A, __m128i B) {
+  // CHECK-LABEL: test_mm_cmpgt_epi16
+  // CHECK: icmp sgt <8 x i16>
+  // CHECK-ASM: pcmpgtw %xmm{{.*}}, %xmm{{.*}}
+  return _mm_cmpgt_epi16(A, B);
+}
+
+__m128i test_mm_cmpgt_epi32(__m128i A, __m128i B) {
+  // CHECK-LABEL: test_mm_cmpgt_epi32
+  // CHECK: icmp sgt <4 x i32>
+  // CHECK-ASM: pcmpgtd %xmm{{.*}}, %xmm{{.*}}
+  return _mm_cmpgt_epi32(A, B);
+}
+
 __m128i test_mm_cmpgt_epi64(__m128i A, __m128i B) {
   // CHECK-LABEL: test_mm_cmpgt_epi64
   // CHECK: icmp sgt <2 x i64>
   // CHECK-ASM: pcmpgtq %xmm{{.*}}, %xmm{{.*}}
   return _mm_cmpgt_epi64(A, B);
 }
 
 int test_mm_cmpestra(__m128i A, int LA, __m128i B, int LB) {