Make the byte-shift SSE intrinsics emit vector shuffles which we know the backend can handle.

[filcab]

Ping!

[craig.topper]

Sorry I missed this earlier.

I'm all for removing unnecessary builtins. Does the backend lower these shuffles to the same instruction the builtin does? Do we have tests for that in the llvm side?

Do you plan to remove the intrinsics from llvm in favor of the shuffles?

The casts inside the diagnostic pragmas seem inconsistent between the two macros. Was that intentional.

Is there already an emmintrin test file this could be added to? Maybe just put them in sse-builtins.c

[filcab]

Hi Craig,

[filcab]

Fixed types in macros.
Removed unused intrinsics.

[filcab]

Sorry about the noise. This one additionaly removes a test that used the __builtin_ function.

[craig.topper]

Can you put the '__extension' back on the same line as the macro name? Other than that LGTM.

[alexr]

Please note that the original patch was developed by Andrea when you land it.

[filcab]

Alex: Thanks for reminding me! He prepared this patch with me and I forgot
to include that in the commit message. Adding it.

Craig: I just want to double-check: clang-format formatted it that way.
Still want me to format it closer to what it was? (since it's an installed
header, you might prefer not strictly following llvm's format)

Thanks,

Filipe

include/clang/Basic/BuiltinsX86.def

 BUILTIN(__builtin_ia32_lfence, "v", "")
 BUILTIN(__builtin_ia32_mfence, "v", "")
 BUILTIN(__builtin_ia32_storedqu, "vc*V16c", "")
 BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "")
 BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "")
 BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "")
 BUILTIN(__builtin_ia32_psrlw128, "V8sV8sV8s", "")
 BUILTIN(__builtin_ia32_psrld128, "V4iV4iV4i", "")
-BUILTIN(__builtin_ia32_pslldqi128, "V2LLiV2LLiIi", "")
-BUILTIN(__builtin_ia32_psrldqi128, "V2LLiV2LLiIi", "")
 BUILTIN(__builtin_ia32_psrlq128, "V2LLiV2LLiV2LLi", "")
 BUILTIN(__builtin_ia32_psllw128, "V8sV8sV8s", "")
 BUILTIN(__builtin_ia32_pslld128, "V4iV4iV4i", "")
 BUILTIN(__builtin_ia32_psllq128, "V2LLiV2LLiV2LLi", "")
 BUILTIN(__builtin_ia32_psllwi128, "V8sV8si", "")
 BUILTIN(__builtin_ia32_pslldi128, "V4iV4ii", "")
 BUILTIN(__builtin_ia32_psllqi128, "V2LLiV2LLii", "")
 BUILTIN(__builtin_ia32_psrlwi128, "V8sV8si", "")

lib/Headers/emmintrin.h

 }
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))
 _mm_xor_si128(__m128i __a, __m128i __b)
 {
   return __a ^ __b;
 }
 
-#define _mm_slli_si128(a, count) __extension__ ({ \
-  _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wshadow\""); \
-  __m128i __a = (a); \
-   _Pragma("clang diagnostic pop"); \
-  (__m128i)__builtin_ia32_pslldqi128(__a, (count)*8); })
+#define _mm_slli_si128(a, imm)                                                 \
+  __extension__({                                                              \
+    _Pragma("clang diagnostic push")                                           \
+        _Pragma("clang diagnostic ignored \"-Wshadow\"");                      \
+    __m128i __a = (a);                                                         \
+    _Pragma("clang diagnostic pop");                                           \
+    (__m128i)                                                                  \
+        __builtin_shufflevector((__v16qi)_mm_setzero_si128(), (__v16qi)__a,    \
+                                ((imm)&0xF0) ? 0 : 16 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 17 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 18 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 19 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 20 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 21 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 22 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 23 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 24 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 25 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 26 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 27 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 28 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 29 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 30 - ((imm)&0xF),           \
+                                ((imm)&0xF0) ? 0 : 31 - ((imm)&0xF));          \
+  })
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))
 _mm_slli_epi16(__m128i __a, int __count)
 {
   return (__m128i)__builtin_ia32_psllwi128((__v8hi)__a, __count);
 }
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))

 }
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))
 _mm_sra_epi32(__m128i __a, __m128i __count)
 {
   return (__m128i)__builtin_ia32_psrad128((__v4si)__a, (__v4si)__count);
 }
 
-
-#define _mm_srli_si128(a, count) __extension__ ({ \
-  _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wshadow\""); \
-  __m128i __a = (a); \
-  _Pragma("clang diagnostic pop"); \
-  (__m128i)__builtin_ia32_psrldqi128(__a, (count)*8); })
+#define _mm_srli_si128(a, imm)                                                 \
+  __extension__({                                                              \
+    _Pragma("clang diagnostic push")                                           \
+        _Pragma("clang diagnostic ignored \"-Wshadow\"");                      \
+    __m128i __a = (a);                                                         \
+    _Pragma("clang diagnostic pop");                                           \
+    (__m128i)                                                                  \
+        __builtin_shufflevector((__v16qi)__a, (__v16qi)_mm_setzero_si128(),    \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 0,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 1,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 2,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 3,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 4,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 5,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 6,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 7,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 8,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 9,           \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 10,          \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 11,          \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 12,          \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 13,          \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 14,          \
+                                ((imm)&0xF0) ? 16 : ((imm)&0xF) + 15);         \
+  })
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))
 _mm_srli_epi16(__m128i __a, int __count)
 {
   return (__m128i)__builtin_ia32_psrlwi128((__v8hi)__a, __count);
 }
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))

test/CodeGen/sse.c

+// RUN: %clang_cc1 -O3 -ffreestanding -triple x86_64-apple-macosx10.8.0 -target-feature +sse4.1 -emit-llvm %s -o - | FileCheck %s
+
+#include <emmintrin.h>
+
+// Byte-shifts look reversed due to xmm register layout
+__m128 test_mm_slli_si128(__m128 a) {
+  // CHECK-LABEL: @test_mm_slli_si128
+  // CHECK: shufflevector <16 x i8> <{{.*}}, i8 0, i8 0, i8 0, i8 0, i8 0>, <16 x i8> {{.*}}, <16 x i32> <i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26>
+  return _mm_slli_si128(a, 5);
+}
+
+__m128 test_mm_slli_si128_0(__m128 a) {
+  // CHECK-LABEL: @test_mm_slli_si128_0
+  // CHECK-NOT: shufflevector
+  return _mm_slli_si128(a, 0);
+}
+
+__m128 test_mm_slli_si128_16(__m128 a) {
+  // CHECK-LABEL: @test_mm_slli_si128_16
+  // CHECK-NOT: shufflevector
+  return _mm_slli_si128(a, 16);
+}
+
+__m128 test_mm_srli_si128(__m128 a) {
+  // CHECK-LABEL: @test_mm_srli_si128
+  // CHECK: shufflevector <16 x i8> {{.*}}, <16 x i8> <i8 0, i8 0, i8 0, i8 0, i8 0, {{.*}}>, <16 x i32> <i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20>
+  return _mm_srli_si128(a, 5);
+}
+
+__m128 test_mm_srli_si128_0(__m128 a) {
+  // CHECK-LABEL: @test_mm_srli_si128_0
+  // CHECK-NOT: shufflevector
+  return _mm_srli_si128(a, 0);
+}
+
+__m128 test_mm_srli_si128_16(__m128 a) {
+  // CHECK-LABEL: @test_mm_srli_si128_16
+  // CHECK-NOT: shufflevector
+  return _mm_srli_si128(a, 16);
+}