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.

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.

Differential Revision: http://reviews.llvm.org/D13324

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

diff --git a/lib/Headers/avx2intrin.h b/lib/Headers/avx2intrin.h
index a5c376cca62cf062380911ff8a9549915bf8a465..90b8530126d00029c8d8c196f59cb4137d5ca268 100644 (file)
--- a/lib/Headers/avx2intrin.h
+++ b/lib/Headers/avx2intrin.h
@@ -208,7 +208,9 @@ _mm256_cmpeq_epi64(__m256i __a, __m256i __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

diff --git a/lib/Headers/avxintrin.h b/lib/Headers/avxintrin.h
index a8418ad0ed6b47f80f172f5de83efa21b2798bb9..7e4de9d443cff1c8279e8cdd912f412b1505157f 100644 (file)
--- a/lib/Headers/avxintrin.h
+++ b/lib/Headers/avxintrin.h
@@ -35,6 +35,10 @@ 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)));

diff --git a/lib/Headers/emmintrin.h b/lib/Headers/emmintrin.h
index 761aefafa18b70a2e26620d42ae087279351977c..47eaf091e1e90ad7706865de9567a5b17f5376c7 100644 (file)
--- a/lib/Headers/emmintrin.h
+++ b/lib/Headers/emmintrin.h
@@ -35,6 +35,10 @@ 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. */
@@ -996,8 +1000,7 @@ 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);
 }
 

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

diff --git a/test/CodeGen/avx2-builtins.c b/test/CodeGen/avx2-builtins.c
index fa74adc3796e06e07ea4f25f856637b9b7d1695d..905728879c4dc34aa90d1cad4b35250d01bfa0a5 100644 (file)
--- a/test/CodeGen/avx2-builtins.c
+++ b/test/CodeGen/avx2-builtins.c
@@ -1,5 +1,7 @@
 // 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
 

diff --git a/test/CodeGen/avx512bw-builtins.c b/test/CodeGen/avx512bw-builtins.c
index a0f25beb3bbd3e3a79032d4f682c919d744a40f3..2878b765417471587e68c60ae960332619a07239 100644 (file)
--- a/test/CodeGen/avx512bw-builtins.c
+++ b/test/CodeGen/avx512bw-builtins.c
@@ -1,4 +1,5 @@
 // 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>
 

diff --git a/test/CodeGen/avx512vlbw-builtins.c b/test/CodeGen/avx512vlbw-builtins.c
index eb12d504a3c846083981fa9a09b789116b10de0e..b78f0d773e6473fd728ca96996dd8910ceb9217d 100644 (file)
--- a/test/CodeGen/avx512vlbw-builtins.c
+++ b/test/CodeGen/avx512vlbw-builtins.c
@@ -1,4 +1,5 @@
 // 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>
 

diff --git a/test/CodeGen/mmx-builtins.c b/test/CodeGen/mmx-builtins.c
index e9f8d8696f9d57b426f3d0ff060b38c24ed3f25d..f17d6eadff094e618f987e0020140679b71c335c 100644 (file)
--- a/test/CodeGen/mmx-builtins.c
+++ b/test/CodeGen/mmx-builtins.c
@@ -1,5 +1,6 @@
 // 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.

diff --git a/test/CodeGen/sse42-builtins.c b/test/CodeGen/sse42-builtins.c
index 6a56df798a1ec9121e83d18ee594d6926524f62f..4b56f12f8d70ab378f05a20413aceba39e9bc6f6 100644 (file)
--- a/test/CodeGen/sse42-builtins.c
+++ b/test/CodeGen/sse42-builtins.c
@@ -1,12 +1,35 @@
 // 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>