Do not define the gcc style target specific pre-defined preprocessor macros in clang-cl

[majnemer]

This patch looks OK, however it doesn't allow users of clang to actually use the builtins; the backend will still error out.

[• ehsan]

That's OK, the main intention behind this patch is to unbreak code that assumes gcc specific things based on #ifdef i386 and friends...

With that in mind, does this look good to land?

[rnk]

How much code breaks without this? Do we really want to do this? My inclination is to get by without it. In particular, libyuv has made changes to not assume that x86_64 implies GNUC functionality.

lib/Basic/Targets.cpp

2592

This condition is incorrect, Microsoft builds apps such as Office for Mac with clang with -fms-extensions. They very likely include system headers that rely on x86_64 &co. This should probably be:

if (MicrosoftExt)
  ... _M_* macros
if (MSVCCompat)
  return;
...

[• ehsan]

In our code base I think it's only occurrence of this problem, but I do
expect this to be the sort of thing that breaks more than just libyuv in
the wild. That being said, I'll leave it up to you whether we should take
this or not, and will address the comment below when landing if we decide
to to do so.

Cheers,

Ehsan
http://ehsanakhgari.org/

lib/Basic/Targets.cpp

else if (SSELevel > NoSSE)
     MMX3DNowLevel = std::max(MMX3DNowLevel, MMX);
   return true;
 }
 
 /// X86TargetInfo::getTargetDefines - Return the set of the X86-specific macro
 /// definitions for this particular subtarget.
 void X86TargetInfo::getTargetDefines(const LangOptions &Opts,
                                      MacroBuilder &Builder) const {
+  // None of these macros make sense in MSVC compat mode.
+  if (Opts.MicrosoftExt) {
+    if (getTriple().getArch() == llvm::Triple::x86) {
+      switch (SSELevel) {
+      case AVX512F:
+      case AVX2:
+      case AVX:
+      case SSE42:
+      case SSE41:
+      case SSSE3:
+      case SSE3:
+      case SSE2:
+        Builder.defineMacro("_M_IX86_FP", Twine(2));
+        break;
+      case SSE1:
+        Builder.defineMacro("_M_IX86_FP", Twine(1));
+        break;
+      default:
+        Builder.defineMacro("_M_IX86_FP", Twine(0));
+      }
+    }
+
+    return;
+  }
+
   // Target identification.
   if (getTriple().getArch() == llvm::Triple::x86_64) {
     Builder.defineMacro("__amd64__");
     Builder.defineMacro("__amd64");
     Builder.defineMacro("__x86_64");
     Builder.defineMacro("__x86_64__");
   } else {
     DefineStd(Builder, "i386", Opts);
case SSE2:
     Builder.defineMacro("__SSE2_MATH__");  // -mfp-math=sse always implied.
   case SSE1:
     Builder.defineMacro("__SSE__");
     Builder.defineMacro("__SSE_MATH__");   // -mfp-math=sse always implied.
   case NoSSE:
     break;
   }
 
-  if (Opts.MicrosoftExt && getTriple().getArch() == llvm::Triple::x86) {
-    switch (SSELevel) {
-    case AVX512F:
-    case AVX2:
-    case AVX:
-    case SSE42:
-    case SSE41:
-    case SSSE3:
-    case SSE3:
-    case SSE2:
-      Builder.defineMacro("_M_IX86_FP", Twine(2));
-      break;
-    case SSE1:
-      Builder.defineMacro("_M_IX86_FP", Twine(1));
-      break;
-    default:
-      Builder.defineMacro("_M_IX86_FP", Twine(0));
-    }
-  }
-
   // Each case falls through to the previous one here.
   switch (MMX3DNowLevel) {
   case AMD3DNowAthlon:
     Builder.defineMacro("__3dNOW_A__");
   case AMD3DNow:
     Builder.defineMacro("__3dNOW__");
   case MMX:
     Builder.defineMacro("__MMX__");

 };
 
 class ARMleTargetInfo : public ARMTargetInfo {
 public:
   ARMleTargetInfo(const llvm::Triple &Triple)
     : ARMTargetInfo(Triple, false) { }
   virtual void getTargetDefines(const LangOptions &Opts,
                                 MacroBuilder &Builder) const {
+    if (!Opts.MSVCCompat)
       Builder.defineMacro("__ARMEL__");
     ARMTargetInfo::getTargetDefines(Opts, Builder);
   }
 };
 
 class ARMbeTargetInfo : public ARMTargetInfo {
 public:
   ARMbeTargetInfo(const llvm::Triple &Triple)
     : ARMTargetInfo(Triple, true) { }
   virtual void getTargetDefines(const LangOptions &Opts,
                                 MacroBuilder &Builder) const {
+    if (!Opts.MSVCCompat) {
       Builder.defineMacro("__ARMEB__");
       Builder.defineMacro("__ARM_BIG_ENDIAN");
+    }
     ARMTargetInfo::getTargetDefines(Opts, Builder);
   }
 };
 } // end anonymous namespace.
 
 namespace {
 class WindowsARMTargetInfo : public WindowsTargetInfo<ARMleTargetInfo> {
   const llvm::Triple Triple;

lib/Headers/Intrin.h

 #ifndef _MSC_VER
 #include_next <Intrin.h>
 #else
 
 #ifndef __INTRIN_H
 #define __INTRIN_H
 
 /* First include the standard intrinsics. */
-#if defined(__i386__) || defined(__x86_64__)
+#if defined(__i386__) || defined(__x86_64__) || defined(_M_IX86) || defined(_M_X64)
 #include <x86intrin.h>
 #endif
 
 /* For the definition of jmp_buf. */
 #include <setjmp.h>
 
 #ifdef __cplusplus
 extern "C" {

lib/Headers/ammintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __AMMINTRIN_H
 #define __AMMINTRIN_H
 
-#ifndef __SSE4A__
+#if !defined(__SSE4A__) && !defined(_MSC_VER)
 #error "SSE4A instruction set not enabled"
 #else
 
 #include <pmmintrin.h>
 
 #define _mm_extracti_si64(x, len, idx) \
   ((__m128i)__builtin_ia32_extrqi((__v2di)(__m128i)(x), \
                                   (char)(len), (char)(idx)))

lib/Headers/arm_acle.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __ARM_ACLE_H
 #define __ARM_ACLE_H
 
-#ifndef __ARM_ACLE
+#if !defined(__ARM_ACLE) && !defined(_MSC_VER)
 #error "ACLE intrinsics support not enabled."
 #endif
 
 #include <stdint.h>
 
 /* Miscellaneous data-processing intrinsics */
 
 static __inline__ uint32_t __attribute__((always_inline, nodebug))

lib/Headers/emmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __EMMINTRIN_H
 #define __EMMINTRIN_H
 
-#ifndef __SSE2__
+#if !defined(__SSE2__) && !defined(_MSC_VER)
 #error "SSE2 instruction set not enabled"
 #else
 
 #include <xmmintrin.h>
 
 typedef double __m128d __attribute__((__vector_size__(16)));
 typedef long long __m128i __attribute__((__vector_size__(16)));
 

lib/Headers/fmaintrin.h

 
 #ifndef __IMMINTRIN_H
 #error "Never use <fmaintrin.h> directly; include <immintrin.h> instead."
 #endif
 
 #ifndef __FMAINTRIN_H
 #define __FMAINTRIN_H
 
-#ifndef __FMA__
+#if !defined(__FMA__) && !defined(_MSC_VER)
 # error "FMA instruction set is not enabled"
 #else
 
 static __inline__ __m128 __attribute__((__always_inline__, __nodebug__))
 _mm_fmadd_ps(__m128 __A, __m128 __B, __m128 __C)
 {
   return (__m128)__builtin_ia32_vfmaddps(__A, __B, __C);
 }

lib/Headers/immintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __IMMINTRIN_H
 #define __IMMINTRIN_H
 
-#ifdef __MMX__
+#if defined(__MMX__) || defined(_MSC_VER)
 #include <mmintrin.h>
 #endif
 
-#ifdef __SSE__
+#if defined(__SSE__) || defined(_MSC_VER)
 #include <xmmintrin.h>
 #endif
 
-#ifdef __SSE2__
+#if defined(__SSE2__) || defined(_MSC_VER)
 #include <emmintrin.h>
 #endif
 
-#ifdef __SSE3__
+#if defined(__SSE3__) || defined(_MSC_VER)
 #include <pmmintrin.h>
 #endif
 
-#ifdef __SSSE3__
+#if defined(__SSSE3__) || defined(_MSC_VER)
 #include <tmmintrin.h>
 #endif
 
-#if defined (__SSE4_2__) || defined (__SSE4_1__)
+#if defined (__SSE4_2__) || defined (__SSE4_1__) || defined(_MSC_VER)
 #include <smmintrin.h>
 #endif
 
-#if defined (__AES__) || defined (__PCLMUL__)
+#if defined (__AES__) || defined (__PCLMUL__) || defined(_MSC_VER)
 #include <wmmintrin.h>
 #endif
 
-#ifdef __AVX__
+#if defined(__AVX__) || defined(_MSC_VER)
 #include <avxintrin.h>
 #endif
 
-#ifdef __AVX2__
+#if defined(__AVX2__) || defined(_MSC_VER)
 #include <avx2intrin.h>
 #endif
 
 #ifdef __BMI__
 #include <bmiintrin.h>
 #endif
 
 #ifdef __BMI2__

lib/Headers/mmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __MMINTRIN_H
 #define __MMINTRIN_H
 
-#ifndef __MMX__
+#if !defined(__MMX__) && !defined(_MSC_VER)
 #error "MMX instruction set not enabled"
 #else
 
 typedef long long __m64 __attribute__((__vector_size__(8)));
 
 typedef int __v2si __attribute__((__vector_size__(8)));
 typedef short __v4hi __attribute__((__vector_size__(8)));
 typedef char __v8qi __attribute__((__vector_size__(8)));

lib/Headers/nmmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef _NMMINTRIN_H
 #define _NMMINTRIN_H
 
-#ifndef __SSE4_2__
+#if !defined(__SSE_4_2__) && !defined(_MSC_VER)
 #error "SSE4.2 instruction set not enabled"
 #else
 
 /* To match expectations of gcc we put the sse4.2 definitions into smmintrin.h,
    just include it now then.  */
 #include <smmintrin.h>
 #endif /* __SSE4_2__ */
 #endif /* _NMMINTRIN_H */

lib/Headers/pmmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
  
 #ifndef __PMMINTRIN_H
 #define __PMMINTRIN_H
 
-#ifndef __SSE3__
+#if !defined(__SSE3__) && !defined(_MSC_VER)
 #error "SSE3 instruction set not enabled"
 #else
 
 #include <emmintrin.h>
 
 static __inline__ __m128i __attribute__((__always_inline__, __nodebug__))
 _mm_lddqu_si128(__m128i const *__p)
 {

lib/Headers/smmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef _SMMINTRIN_H
 #define _SMMINTRIN_H
 
-#ifndef __SSE4_1__
+#if !defined(__SSE4_1__) && !defined(_MSC_VER)
 #error "SSE4.1 instruction set not enabled"
 #else
 
 #include <tmmintrin.h>
 
 /* SSE4 Rounding macros. */
 #define _MM_FROUND_TO_NEAREST_INT    0x00
 #define _MM_FROUND_TO_NEG_INF        0x01

lib/Headers/tmmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
  
 #ifndef __TMMINTRIN_H
 #define __TMMINTRIN_H
 
-#ifndef __SSSE3__
+#if !defined(__SSE3__) && !defined(_MSC_VER)
 #error "SSSE3 instruction set not enabled"
 #else
 
 #include <pmmintrin.h>
 
 static __inline__ __m64 __attribute__((__always_inline__, __nodebug__))
 _mm_abs_pi8(__m64 __a)
 {

lib/Headers/wmmintrin.h

  *===-----------------------------------------------------------------------===
  */
 
 #ifndef _WMMINTRIN_H
 #define _WMMINTRIN_H
 
 #include <emmintrin.h>
 
-#if !defined (__AES__) && !defined (__PCLMUL__)
+#if !defined (__AES__) && !defined (__PCLMUL__) && !defined (_MSC_VER)
 # error "AES/PCLMUL instructions not enabled"
 #else
 
 #ifdef __AES__
 #include <__wmmintrin_aes.h>
 #endif /* __AES__ */
 
 #ifdef __PCLMUL__
 #include <__wmmintrin_pclmul.h>
 #endif /* __PCLMUL__ */
 
 #endif /* __AES__ || __PCLMUL__ */
 #endif /* _WMMINTRIN_H */

lib/Headers/x86intrin.h

 
 #ifndef __X86INTRIN_H
 #define __X86INTRIN_H
 
 #include <ia32intrin.h>
 
 #include <immintrin.h>
 
-#ifdef __3dNOW__
+#if defined(__3dNOW__) || defined(_MSC_VER)
 #include <mm3dnow.h>
 #endif
 
 #ifdef __BMI__
 #include <bmiintrin.h>
 #endif
 
 #ifdef __BMI2__

 #ifdef __RDSEED__
 #include <rdseedintrin.h>
 #endif
 
 #ifdef __PRFCHW__
 #include <prfchwintrin.h>
 #endif
 
-#ifdef __SSE4A__
+#if defined(_SSE4A__) || defined(_MSC_VER)
 #include <ammintrin.h>
 #endif
 
 #ifdef __FMA4__
 #include <fma4intrin.h>
 #endif
 
 #ifdef __XOP__

lib/Headers/xmmintrin.h

  * THE SOFTWARE.
  *
  *===-----------------------------------------------------------------------===
  */
  
 #ifndef __XMMINTRIN_H
 #define __XMMINTRIN_H
  
-#ifndef __SSE__
+#if !defined(__SSE__) && !defined(_MSC_VER)
 #error "SSE instruction set not enabled"
 #else
 
 #include <mmintrin.h>
 
 typedef int __v4si __attribute__((__vector_size__(16)));
 typedef float __v4sf __attribute__((__vector_size__(16)));
 typedef float __m128 __attribute__((__vector_size__(16)));

test/CodeGenCXX/mangle-ms-vector-types.cpp

-// RUN: %clang_cc1 -fms-extensions -ffreestanding -target-feature +avx -emit-llvm %s -o - -triple=i686-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -fms-extensions -fmsc-version=1700 -ffreestanding -target-feature +avx -emit-llvm %s -o - -triple=i686-pc-win32 | FileCheck %s
 
 #include <xmmintrin.h>
 #include <emmintrin.h>
 #include <immintrin.h>
 
 void foo64(__m64) {}
 // CHECK: define void @"\01?foo64@@YAXT__m64@@@Z"
 

test/Preprocessor/predefined-macros.c

 // This test verifies that the correct macros are predefined.
 //
 // RUN: %clang_cc1 %s -E -dM -triple i686-pc-win32 -fms-extensions -fms-compatibility \
 // RUN:     -fmsc-version=1300 -o - | FileCheck %s --check-prefix=CHECK-MS
 // CHECK-MS: #define _INTEGRAL_MAX_BITS 64
 // CHECK-MS: #define _MSC_EXTENSIONS 1
 // CHECK-MS: #define _MSC_VER 1300
 // CHECK-MS: #define _M_IX86 600
 // CHECK-MS: #define _M_IX86_FP
 // CHECK-MS: #define _WIN32 1
 // CHECK-MS-NOT: #define __GNUC__
 // CHECK-MS-NOT: #define __STRICT_ANSI__
+// CHECK-MS-NOT: #define __i386__
+// CHECK-MS-NOT: #define __i486__
+// CHECK-MS-NOT: #define __i686__
+// CHECK-MS-NOT: #define __SSE__
+//
+// RUN: %clang_cc1 %s -E -dM -triple x86_64-pc-win32 -fms-extensions -fms-compatibility \
+// RUN:     -fmsc-version=1300 -o - | FileCheck %s --check-prefix=CHECK-MS64
+// CHECK-MS64: #define _INTEGRAL_MAX_BITS 64
+// CHECK-MS64: #define _MSC_EXTENSIONS 1
+// CHECK-MS64: #define _MSC_VER 1300
+// CHECK-MS64: #define _M_AMD64 1
+// CHECK-MS64: #define _M_X64 1
+// CHECK-MS64: #define _WIN64 1
+// CHECK-MS64-NOT: #define __GNUC__
+// CHECK-MS64-NOT: #define __STRICT_ANSI__
+// CHECK-MS64-NOT: #define __i486__
+// CHECK-MS64-NOT: #define __i686__
+// CHECK-MS64-NOT: #define __i386__
+// CHECK-MS64-NOT: #define __SSE__
+// CHECK-MS64-NOT: #define __x86_64__
+// CHECK-MS64-NOT: #define __amd64__
 //
 // RUN: %clang_cc1 %s -E -dM -triple i686-pc-win32 -fms-compatibility \
 // RUN:     -o - | FileCheck %s --check-prefix=CHECK-MS-STDINT
 // CHECK-MS-STDINT-NOT:#define __INT16_MAX__ 32767
 // CHECK-MS-STDINT-NOT:#define __INT32_MAX__ 2147483647
 // CHECK-MS-STDINT-NOT:#define __INT64_MAX__ 9223372036854775807LL
 // CHECK-MS-STDINT-NOT:#define __INT8_MAX__ 127
 // CHECK-MS-STDINT-NOT:#define __INTPTR_MAX__ 2147483647