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

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

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 ↗

(On Diff #10568)

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 ↗

(On Diff #10568)

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_longjmp: {
     // longjmp doesn't return; mark this as unreachable.
     Builder.CreateUnreachable();
 
     // We do need to preserve an insertion point.
     EmitBlock(createBasicBlock("longjmp.cont"));
 
     return RValue::get(nullptr);
   }
+  case Builtin::BI_InterlockedExchange:
+  case Builtin::BI_InterlockedExchangePointer:
+  case Builtin::BI_InterlockedCompareExchangePointer:
   case Builtin::BI__sync_fetch_and_add:
   case Builtin::BI__sync_fetch_and_sub:
   case Builtin::BI__sync_fetch_and_or:
   case Builtin::BI__sync_fetch_and_and:
   case Builtin::BI__sync_fetch_and_xor:
   case Builtin::BI__sync_add_and_fetch:
   case Builtin::BI__sync_sub_and_fetch:
   case Builtin::BI__sync_and_and_fetch:
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_InterlockedCompareExchange: {
     AtomicCmpXchgInst *CXI = Builder.CreateAtomicCmpXchg(
         EmitScalarExpr(E->getArg(0)),
         EmitScalarExpr(E->getArg(2)),
         EmitScalarExpr(E->getArg(1)),
         SequentiallyConsistent,
         SequentiallyConsistent);
       CXI->setVolatile(true);
       return RValue::get(CXI);
   }
   case Builtin::BI_InterlockedIncrement: {
     AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
       AtomicRMWInst::Add,
       EmitScalarExpr(E->getArg(0)),
       ConstantInt::get(Int32Ty, 1),
       llvm::SequentiallyConsistent);
     RMWI->setVolatile(true);
     return RValue::get(Builder.CreateAdd(RMWI, ConstantInt::get(Int32Ty, 1)));
   }
   case Builtin::BI_InterlockedDecrement: {
     AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
       AtomicRMWInst::Sub,
       EmitScalarExpr(E->getArg(0)),
       ConstantInt::get(Int32Ty, 1),
       llvm::SequentiallyConsistent);
     RMWI->setVolatile(true);
     return RValue::get(Builder.CreateSub(RMWI, ConstantInt::get(Int32Ty, 1)));

lib/Sema/SemaChecking.cpp

for (unsigned ArgNo = 0; ICEArguments != 0; ++ArgNo) {
     // Skip arguments not required to be ICE's.
     if ((ICEArguments & (1 << ArgNo)) == 0) continue;
     
     llvm::APSInt Result;
     if (SemaBuiltinConstantArg(TheCall, ArgNo, Result))
       return true;
     ICEArguments &= ~(1 << ArgNo);
   }
-  
   switch (BuiltinID) {
+  case Builtin::BI_InterlockedExchange:
+  case Builtin::BI_InterlockedExchangePointer:
+    BuiltinID = Builtin::BI__sync_lock_test_and_set;
+    break;
+  case Builtin::BI_InterlockedCompareExchangePointer:
+    BuiltinID = Builtin::BI__sync_val_compare_and_swap;
+
+    assert(TheCall->getNumArgs() == 3);
+    Expr *Arg1 = TheCall->getArg(1), *Arg2 = TheCall->getArg(2);
+    TheCall->setArg(1, Arg2);
+    TheCall->setArg(2, Arg1);
+
+    break;
+  }
+  
+  switch (BuiltinID) {
   case Builtin::BI__builtin___CFStringMakeConstantString:
     assert(TheCall->getNumArgs() == 1 &&
            "Wrong # arguments to builtin CFStringMakeConstantString");
     if (CheckObjCString(TheCall->getArg(0)))
       return ExprError();
     break;
   case Builtin::BI__builtin_stdarg_start:
   case Builtin::BI__builtin_va_start:
#undef BUILTIN_ROW
   case Builtin::BI__sync_xor_and_fetch_1:
   case Builtin::BI__sync_xor_and_fetch_2:
   case Builtin::BI__sync_xor_and_fetch_4:
   case Builtin::BI__sync_xor_and_fetch_8:
   case Builtin::BI__sync_xor_and_fetch_16:
     BuiltinIndex = 9; 
     break;
 
+  case Builtin::BI_InterlockedCompareExchangePointer:
   case Builtin::BI__sync_val_compare_and_swap:
   case Builtin::BI__sync_val_compare_and_swap_1:
   case Builtin::BI__sync_val_compare_and_swap_2:
   case Builtin::BI__sync_val_compare_and_swap_4:
   case Builtin::BI__sync_val_compare_and_swap_8:
   case Builtin::BI__sync_val_compare_and_swap_16:
     BuiltinIndex = 10;
     NumFixed = 2;
     break;
       
   case Builtin::BI__sync_bool_compare_and_swap:
   case Builtin::BI__sync_bool_compare_and_swap_1:
   case Builtin::BI__sync_bool_compare_and_swap_2:
   case Builtin::BI__sync_bool_compare_and_swap_4:
   case Builtin::BI__sync_bool_compare_and_swap_8:
   case Builtin::BI__sync_bool_compare_and_swap_16:
     BuiltinIndex = 11;
     NumFixed = 2;
     ResultType = Context.BoolTy;
     break;
       
+  case Builtin::BI_InterlockedExchange:
+  case Builtin::BI_InterlockedExchangePointer:
   case Builtin::BI__sync_lock_test_and_set: 
   case Builtin::BI__sync_lock_test_and_set_1:
   case Builtin::BI__sync_lock_test_and_set_2:
   case Builtin::BI__sync_lock_test_and_set_4:
   case Builtin::BI__sync_lock_test_and_set_8:
   case Builtin::BI__sync_lock_test_and_set_16:
     BuiltinIndex = 12; 
     break;

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 *Comperand) {
+  return _InterlockedCompareExchangePointer(Destination, Exchange, Comperand);
+}
+
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer(i8** %Destination, i8* %Exchange, i8* %Comperand){{.*}}{
+// CHECK: entry:
+// CHECK:   %0 = bitcast i8** %Destination to i32*
+// CHECK:   %1 = ptrtoint i8* %Comperand to i32
+// CHECK:   %2 = ptrtoint i8* %Exchange to i32
+// CHECK:   %3 = cmpxchg i32* %0, i32 %1, i32 %2 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: }