CodeGen: improve ms instrincics support

[abdulras]

Summary

Add support for _InterlockedCompareExchangePointer, _InterlockExchangePointer,
_InterlockExchange. These are available as a compiler intrinsic on ARM and x86.
These are used directly by the Windows SDK headers without use of the intrin
header.

[rnk]

lib/Sema/SemaChecking.cpp

139 ↗

(On Diff #10512)

I don't think these intrinsics need the special overload handling that the __sync intrinsics require. They should just need type checking that's already described by Builtins.def.

145–147 ↗

(On Diff #10512)

The AST should reflect the code the user wrote, and we shouldn't be doing this kind of rewrite. The overload resolution in the existing checking code is different, it's taking something that the user wrote and annotating it with all the implicit conversions that should occur.

This should have it's own lowering in CGBuiltin.cpp.

[abdulras]

Address comments from rnk.

[whunt]

lib/Headers/Intrin.h

227

I don't see why it was necessary to get rid of cdecl. Also, Now that the definition is gone it shouldn't be static inline__ anymore.

[abdulras]

lib/Headers/Intrin.h

227

Getting rid of __cdecl is actually quite important since this header is not x86 specific. ARM does not honour __cdecl, and thus generates a warning.

Good catch about the static __inline__. I've become too accustomed to the LLVM style and didnt notice that.

[abdulras]

Fix Intrin.h with minor oversights caught by Warren.

[whunt]

lgtm

[rnk]

lgtm with some nits

include/clang/Basic/Builtins.def

690

MSVC doesn't provide this in 32-bit mode, but I think we can go ahead and do that anyway.

lib/CodeGen/CGBuiltin.cpp

1523

I'd like this to be factored better, but I can live with it as is.

1535

CGM.Int32Ty doesn't seem correct for x64, it should be IntType.

1537

MSDN says "Comparand" with an 'a'.

1540

MSDN says the 'Dest' is volatile, so I'd call Result->setVolatile(true) here.

[abdulras]

include/clang/Basic/Builtins.def

690

Id be willing to do a follow up change to disable this at the sema level in 32-bit x86 only if you like. This is available in 32-bit ARM.

lib/CodeGen/CGBuiltin.cpp

1535

Good catch!

1537

Done.

1540

Dest itself is volatile, the returned value is not. We dont strip volatility off of Destination. Oh, and I dont see a llvm::Value::setVolatile either.

[rnk]

include/clang/Basic/Builtins.def

690

No need. I always thought it was silly that they didn't go back and add this intrinsic for x86.

lib/CodeGen/CGBuiltin.cpp

1540

CreateAtomicCmpXchg should return an AtomicRMWInst* which can be made volatile. It's not the result that's volatile, it's the memory update.

[abdulras]

lib/CodeGen/CGBuiltin.cpp

1540

Done (thanks for the clarification!); although needed an additional extract value due to a recent change to the cmpxchg change.

include/clang/Basic/Builtins.def

 
 // Microsoft builtins.  These are only active with -fms-extensions.
 LANGBUILTIN(_alloca,      "v*z", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__assume,     "vb",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,       "v.",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__debugbreak, "v",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__va_start,   "vc**.", "nt", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedCompareExchange, "LiLiD*LiLi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedCompareExchangePointer, "v*v*D*v*v*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedIncrement, "LiLiD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedDecrement, "LiLiD*", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedExchangeAdd, "LiLiD*Li", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedExchangePointer, "v*v*D*v*", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_InterlockedExchange, "LiLiD*Li", "n", ALL_MS_LANGUAGES)
 
 // C99 library functions
 // C99 stdlib.h
 LIBBUILTIN(abort, "v",            "fr",    "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(calloc, "v*zz",        "f",     "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(exit, "vi",            "fr",    "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(_Exit, "vi",           "fr",    "stdlib.h", ALL_LANGUAGES)
 LIBBUILTIN(malloc, "v*z",         "f",     "stdlib.h", ALL_LANGUAGES)

lib/CodeGen/CGBuiltin.cpp

   case Builtin::BI__builtin_operator_new:
     return EmitBuiltinNewDeleteCall(FD->getType()->castAs<FunctionProtoType>(),
                                     E->getArg(0), false);
   case Builtin::BI__builtin_operator_delete:
     return EmitBuiltinNewDeleteCall(FD->getType()->castAs<FunctionProtoType>(),
                                     E->getArg(0), true);
   case Builtin::BI__noop:
     return RValue::get(nullptr);
+  case Builtin::BI_InterlockedExchange:
+  case Builtin::BI_InterlockedExchangePointer: {
+    return EmitBinaryAtomic(*this, llvm::AtomicRMWInst::Xchg, E);
+  }
+  case Builtin::BI_InterlockedCompareExchangePointer: {
+    llvm::Type *RTy;
+    llvm::IntegerType *IntType =
+      IntegerType::get(getLLVMContext(),
+                       getContext().getTypeSize(E->getType()));
+    llvm::Type *IntPtrType = IntType->getPointerTo();
+
+    llvm::Value *Destination =
+      Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), IntPtrType);
+
+    llvm::Value *Exchange = EmitScalarExpr(E->getArg(1));
+    RTy = Exchange->getType();
+    Exchange = Builder.CreatePtrToInt(Exchange, CGM.Int32Ty);
+
+    llvm::Value *Comparend =
+      Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), CGM.Int32Ty);
+
+    Value *Result = Builder.CreateAtomicCmpXchg(Destination, Comparend,
+                                                Exchange,
+                                                SequentiallyConsistent,
+                                                SequentiallyConsistent);
+    Result = Builder.CreateIntToPtr(Result, RTy);
+    return RValue::get(Result);
+  }
   case Builtin::BI_InterlockedCompareExchange: {
     AtomicCmpXchgInst *CXI = Builder.CreateAtomicCmpXchg(
         EmitScalarExpr(E->getArg(0)),
         EmitScalarExpr(E->getArg(2)),
         EmitScalarExpr(E->getArg(1)),
         SequentiallyConsistent,
         SequentiallyConsistent);
       CXI->setVolatile(true);

lib/Headers/Intrin.h

 void *_InterlockedCompareExchangePointer_HLEAcquire(void *volatile *, void *,
                                                     void *);
 void *_InterlockedCompareExchangePointer_HLERelease(void *volatile *, void *,
                                                     void *);
 static __inline__
 long __cdecl _InterlockedDecrement(long volatile *_Addend);
 static __inline__
 short _InterlockedDecrement16(short volatile *_Addend);
-static __inline__
-long __cdecl _InterlockedExchange(long volatile *_Target, long _Value);
+long _InterlockedExchange(long volatile *_Target, long _Value);
 static __inline__
 short _InterlockedExchange16(short volatile *_Target, short _Value);
 static __inline__
 char _InterlockedExchange8(char volatile *_Target, char _Value);
 static __inline__
 long __cdecl _InterlockedExchangeAdd(long volatile *_Addend, long _Value);
 long _InterlockedExchangeAdd_HLEAcquire(long volatile *, long);
 long _InterlockedExchangeAdd_HLERelease(long volatile *, long);

 short _InterlockedCompareExchange16_np(short volatile *_Destination,
                                        short _Exchange, short _Comparand);
 __int64 _InterlockedCompareExchange64_HLEAcquire(__int64 volatile *, __int64,
                                                  __int64);
 __int64 _InterlockedCompareExchange64_HLERelease(__int64 volatile *, __int64,
                                                  __int64);
 __int64 _InterlockedCompareExchange64_np(__int64 volatile *_Destination,
                                          __int64 _Exchange, __int64 _Comparand);
-static __inline__
 void *_InterlockedCompareExchangePointer(void *volatile *_Destination,
                                          void *_Exchange, void *_Comparand);
 void *_InterlockedCompareExchangePointer_np(void *volatile *_Destination,
                                             void *_Exchange, void *_Comparand);
 static __inline__
 __int64 _InterlockedDecrement64(__int64 volatile *_Addend);
 static __inline__
 __int64 _InterlockedExchange64(__int64 volatile *_Target, __int64 _Value);
 static __inline__
 __int64 _InterlockedExchangeAdd64(__int64 volatile *_Addend, __int64 _Value);
-static __inline__
 void *_InterlockedExchangePointer(void *volatile *_Target, void *_Value);
 static __inline__
 __int64 _InterlockedIncrement64(__int64 volatile *_Addend);
 long _InterlockedOr_np(long volatile *_Value, long _Mask);
 short _InterlockedOr16_np(short volatile *_Value, short _Mask);
 static __inline__
 __int64 _InterlockedOr64(__int64 volatile *_Value, __int64 _Mask);
 __int64 _InterlockedOr64_np(__int64 volatile *_Value, __int64 _Mask);
_InterlockedExchange8(char volatile *_Target, char _Value) {
   __atomic_exchange(_Target, &_Value, &_Value, 0);
   return _Value;
 }
 static __inline__ short __attribute__((__always_inline__, __nodebug__))
 _InterlockedExchange16(short volatile *_Target, short _Value) {
   __atomic_exchange(_Target, &_Value, &_Value, 0);
   return _Value;
 }
-static __inline__ long __attribute__((__always_inline__, __nodebug__))
-_InterlockedExchange(long volatile *_Target, long _Value) {
-  __atomic_exchange(_Target, &_Value, &_Value, 0);
-  return _Value;
-}
 #ifdef __x86_64__
 static __inline__ __int64 __attribute__((__always_inline__, __nodebug__))
 _InterlockedExchange64(__int64 volatile *_Target, __int64 _Value) {
   __atomic_exchange(_Target, &_Value, &_Value, 0);
   return _Value;
 }
-static __inline__ void *__attribute__((__always_inline__, __nodebug__))
-_InterlockedExchangePointer(void *volatile *_Target, void *_Value) {
-  __atomic_exchange(_Target, &_Value, &_Value, 0);
-  return _Value;
-}
 #endif
 /*----------------------------------------------------------------------------*\
 |* Interlocked Compare Exchange
 \*----------------------------------------------------------------------------*/
 static __inline__ char __attribute__((__always_inline__, __nodebug__))
 _InterlockedCompareExchange8(char volatile *_Destination,
                              char _Exchange, char _Comparand) {
   __atomic_compare_exchange(_Destination, &_Comparand, &_Exchange, 0, 0, 0);
   return _Comparand;
 }
 static __inline__ short __attribute__((__always_inline__, __nodebug__))
 _InterlockedCompareExchange16(short volatile *_Destination,
                               short _Exchange, short _Comparand) {
   __atomic_compare_exchange(_Destination, &_Comparand, &_Exchange, 0, 0, 0);
   return _Comparand;
 }
-#ifdef __x86_64__
-static __inline__ void *__attribute__((__always_inline__, __nodebug__))
-_InterlockedCompareExchangePointer(void *volatile *_Destination,
-                                   void *_Exchange, void *_Comparand) {
-  __atomic_compare_exchange(_Destination, &_Comparand, &_Exchange, 0, 0, 0);
-  return _Comparand;
-}
-#endif
 static __inline__ __int64 __attribute__((__always_inline__, __nodebug__))
 _InterlockedCompareExchange64(__int64 volatile *_Destination,
                               __int64 _Exchange, __int64 _Comparand) {
   __atomic_compare_exchange(_Destination, &_Comparand, &_Exchange, 0, 0, 0);
   return _Comparand;
 }
 /*----------------------------------------------------------------------------*\
 |* Barriers

test/CodeGen/ms-intrinsics.c

+// RUN: %clang_cc1 -triple i686--windows -fms-compatibility -Oz -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple thumbv7--windows -fms-compatibility -Oz -emit-llvm %s -o - | FileCheck %s
+
+void *test_InterlockedExchangePointer(void * volatile *Target, void *Value) {
+  return _InterlockedExchangePointer(Target, Value);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedExchangePointer(i8** %Target, i8* %Value){{.*}}{
+// CHECK: entry:
+// CHECK:   %0 = bitcast i8** %Target to i32*
+// CHECK:   %1 = ptrtoint i8* %Value to i32
+// CHECK:   %2 = atomicrmw xchg i32* %0, i32 %1 seq_cst
+// CHECK:   %3 = inttoptr i32 %2 to i8*
+// CHECK:   ret i8* %3
+// CHECK: }
+
+void *test_InterlockedCompareExchangePointer(void * volatile *Destination,
+                                             void *Exchange, void *Comparend) {
+  return _InterlockedCompareExchangePointer(Destination, Exchange, Comparend);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer(i8** %Destination, i8* %Exchange, i8* %Comparend){{.*}}{
+// CHECK: entry:
+// CHECK:   %0 = bitcast i8** %Destination to i32*
+// CHECK:   %1 = ptrtoint i8* %Exchange to i32
+// CHECK:   %2 = ptrtoint i8* %Comparend to i32
+// CHECK:   %3 = cmpxchg i32* %0, i32 %2, i32 %1 seq_cst seq_cst
+// CHECK:   %4 = inttoptr i32 %3 to i8*
+// CHECK:   ret i8* %4
+// CHECK: }
+
+long test_InterlockedExchange(long *Target, long Value) {
+  return _InterlockedExchange(Target, Value);
+}
+
+// CHECK: define{{.*}}i32 @test_InterlockedExchange(i32* %Target, i32 %Value){{.*}}{
+// CHECK: entry:
+// CHECK:   %0 = atomicrmw xchg i32* %Target, i32 %Value seq_cst
+// CHECK:   ret i32 %0
+// CHECK: }