<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 20, 2014 at 1:38 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="ltr">Remember how I said it'd be OK if we provided _InterlockedCompareExchangePointer on x86? Well, doing that broke the sanitizers. Do 'check-asan' on 32-bit Windows to reproduce.<div><br></div><div>
This is the problematic code:</div><div><div>inline static void *_InterlockedCompareExchangePointer(<br></div><div> void *volatile *Destination,</div><div> void *Exchange, void *Comparand) {</div><div> return reinterpret_cast<void*>(</div>
<div> _InterlockedCompareExchange(</div><div> reinterpret_cast<long volatile*>(Destination), // NOLINT</div><div> reinterpret_cast<long>(Exchange), // NOLINT</div><div> reinterpret_cast<long>(Comparand))); // NOLINT</div>
<div>}</div></div></div></blockquote><div><br></div><div>Ugh, seems that my message never got sent, it remained in the drafts :-(. Thanks for submitting a fix for it Timur. I had the following conflict which is why it never got addressed:</div>
<div><br></div><div>I seem to be having issues building check-asan, as in missing the target :-(. However, since you were kind enough to provide the reduction of the issue, it really helps.</div>
<div><br></div><div>Im conflicted on the best approach to solve this. We could either</div><div>a) filter this in FunctionDecl::getBuiltinID</div><div>b) not provide this in ASAN.</div><div><br></div><div>AIUI, ASAN does require instrumentation of code, so it would not be possible to actually build ASAN enabled code reasonably with Visual Studio at the moment. If I am mistaken on this point, Id love to know about that, and in that case, we could wrap this in a #if !defined(__clang__) check.</div>
<div><br></div><div>Are there other similar builtins that we may have to isolate to specific platforms? If so, it might be worth the effort to build up infrastructure to handle this generically.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 18, 2014 at 1:51 PM, Saleem Abdulrasool <span dir="ltr"><<a href="mailto:compnerd@compnerd.org" target="_blank">compnerd@compnerd.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: compnerd<br>
Date: Wed Jun 18 15:51:10 2014<br>
New Revision: 211216<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=211216&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=211216&view=rev</a><br>
Log:<br>
CodeGen: improve ms instrincics support<br>
<br>
Add support for _InterlockedCompareExchangePointer, _InterlockExchangePointer,<br>
_InterlockExchange. These are available as a compiler intrinsic on ARM and x86.<br>
These are used directly by the Windows SDK headers without use of the intrin<br>
header.<br>
<br>
Added:<br>
cfe/trunk/test/CodeGen/ms-intrinsics.c<br>
Modified:<br>
cfe/trunk/include/clang/Basic/Builtins.def<br>
cfe/trunk/lib/CodeGen/CGBuiltin.cpp<br>
cfe/trunk/lib/Headers/Intrin.h<br>
<br>
Modified: cfe/trunk/include/clang/Basic/Builtins.def<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=211216&r1=211215&r2=211216&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=211216&r1=211215&r2=211216&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/Builtins.def (original)<br>
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Jun 18 15:51:10 2014<br>
@@ -683,9 +683,12 @@ LANGBUILTIN(__noop, "v.", "n", AL<br>
LANGBUILTIN(__debugbreak, "v", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(__va_start, "vc**.", "nt", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_InterlockedCompareExchange, "LiLiD*LiLi", "n", ALL_MS_LANGUAGES)<br>
+LANGBUILTIN(_InterlockedCompareExchangePointer, "v*v*D*v*v*", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_InterlockedIncrement, "LiLiD*", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_InterlockedDecrement, "LiLiD*", "n", ALL_MS_LANGUAGES)<br>
LANGBUILTIN(_InterlockedExchangeAdd, "LiLiD*Li", "n", ALL_MS_LANGUAGES)<br>
+LANGBUILTIN(_InterlockedExchangePointer, "v*v*D*v*", "n", ALL_MS_LANGUAGES)<br>
+LANGBUILTIN(_InterlockedExchange, "LiLiD*Li", "n", ALL_MS_LANGUAGES)<br>
<br>
// C99 library functions<br>
// C99 stdlib.h<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=211216&r1=211215&r2=211216&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=211216&r1=211215&r2=211216&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jun 18 15:51:10 2014<br>
@@ -1516,6 +1516,35 @@ RValue CodeGenFunction::EmitBuiltinExpr(<br>
E->getArg(0), true);<br>
case Builtin::BI__noop:<br>
return RValue::get(nullptr);<br>
+ case Builtin::BI_InterlockedExchange:<br>
+ case Builtin::BI_InterlockedExchangePointer:<br>
+ return EmitBinaryAtomic(*this, llvm::AtomicRMWInst::Xchg, E);<br>
+ case Builtin::BI_InterlockedCompareExchangePointer: {<br>
+ llvm::Type *RTy;<br>
+ llvm::IntegerType *IntType =<br>
+ IntegerType::get(getLLVMContext(),<br>
+ getContext().getTypeSize(E->getType()));<br>
+ llvm::Type *IntPtrType = IntType->getPointerTo();<br>
+<br>
+ llvm::Value *Destination =<br>
+ Builder.CreateBitCast(EmitScalarExpr(E->getArg(0)), IntPtrType);<br>
+<br>
+ llvm::Value *Exchange = EmitScalarExpr(E->getArg(1));<br>
+ RTy = Exchange->getType();<br>
+ Exchange = Builder.CreatePtrToInt(Exchange, IntType);<br>
+<br>
+ llvm::Value *Comparand =<br>
+ Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType);<br>
+<br>
+ auto Result = Builder.CreateAtomicCmpXchg(Destination, Comparand, Exchange,<br>
+ SequentiallyConsistent,<br>
+ SequentiallyConsistent);<br>
+ Result->setVolatile(true);<br>
+<br>
+ return RValue::get(Builder.CreateIntToPtr(Builder.CreateExtractValue(Result,<br>
+ 0),<br>
+ RTy));<br>
+ }<br>
case Builtin::BI_InterlockedCompareExchange: {<br>
AtomicCmpXchgInst *CXI = Builder.CreateAtomicCmpXchg(<br>
EmitScalarExpr(E->getArg(0)),<br>
<br>
Modified: cfe/trunk/lib/Headers/Intrin.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/Intrin.h?rev=211216&r1=211215&r2=211216&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/Intrin.h?rev=211216&r1=211215&r2=211216&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Headers/Intrin.h (original)<br>
+++ cfe/trunk/lib/Headers/Intrin.h Wed Jun 18 15:51:10 2014<br>
@@ -223,8 +223,7 @@ static __inline__<br>
long __cdecl _InterlockedDecrement(long volatile *_Addend);<br>
static __inline__<br>
short _InterlockedDecrement16(short volatile *_Addend);<br>
-static __inline__<br>
-long __cdecl _InterlockedExchange(long volatile *_Target, long _Value);<br>
+long _InterlockedExchange(long volatile *_Target, long _Value);<br>
static __inline__<br>
short _InterlockedExchange16(short volatile *_Target, short _Value);<br>
static __inline__<br>
@@ -411,7 +410,6 @@ __int64 _InterlockedCompareExchange64_HL<br>
__int64);<br>
__int64 _InterlockedCompareExchange64_np(__int64 volatile *_Destination,<br>
__int64 _Exchange, __int64 _Comparand);<br>
-static __inline__<br>
void *_InterlockedCompareExchangePointer(void *volatile *_Destination,<br>
void *_Exchange, void *_Comparand);<br>
void *_InterlockedCompareExchangePointer_np(void *volatile *_Destination,<br>
@@ -422,7 +420,6 @@ static __inline__<br>
__int64 _InterlockedExchange64(__int64 volatile *_Target, __int64 _Value);<br>
static __inline__<br>
__int64 _InterlockedExchangeAdd64(__int64 volatile *_Addend, __int64 _Value);<br>
-static __inline__<br>
void *_InterlockedExchangePointer(void *volatile *_Target, void *_Value);<br>
static __inline__<br>
__int64 _InterlockedIncrement64(__int64 volatile *_Addend);<br>
@@ -785,22 +782,12 @@ _InterlockedExchange16(short volatile *_<br>
__atomic_exchange(_Target, &_Value, &_Value, 0);<br>
return _Value;<br>
}<br>
-static __inline__ long __attribute__((__always_inline__, __nodebug__))<br>
-_InterlockedExchange(long volatile *_Target, long _Value) {<br>
- __atomic_exchange(_Target, &_Value, &_Value, 0);<br>
- return _Value;<br>
-}<br>
#ifdef __x86_64__<br>
static __inline__ __int64 __attribute__((__always_inline__, __nodebug__))<br>
_InterlockedExchange64(__int64 volatile *_Target, __int64 _Value) {<br>
__atomic_exchange(_Target, &_Value, &_Value, 0);<br>
return _Value;<br>
}<br>
-static __inline__ void *__attribute__((__always_inline__, __nodebug__))<br>
-_InterlockedExchangePointer(void *volatile *_Target, void *_Value) {<br>
- __atomic_exchange(_Target, &_Value, &_Value, 0);<br>
- return _Value;<br>
-}<br>
#endif<br>
/*----------------------------------------------------------------------------*\<br>
|* Interlocked Compare Exchange<br>
@@ -817,14 +804,6 @@ _InterlockedCompareExchange16(short vola<br>
__atomic_compare_exchange(_Destination, &_Comparand, &_Exchange, 0, 0, 0);<br>
return _Comparand;<br>
}<br>
-#ifdef __x86_64__<br>
-static __inline__ void *__attribute__((__always_inline__, __nodebug__))<br>
-_InterlockedCompareExchangePointer(void *volatile *_Destination,<br>
- void *_Exchange, void *_Comparand) {<br>
- __atomic_compare_exchange(_Destination, &_Comparand, &_Exchange, 0, 0, 0);<br>
- return _Comparand;<br>
-}<br>
-#endif<br>
static __inline__ __int64 __attribute__((__always_inline__, __nodebug__))<br>
_InterlockedCompareExchange64(__int64 volatile *_Destination,<br>
__int64 _Exchange, __int64 _Comparand) {<br>
<br>
Added: cfe/trunk/test/CodeGen/ms-intrinsics.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics.c?rev=211216&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics.c?rev=211216&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGen/ms-intrinsics.c (added)<br>
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c Wed Jun 18 15:51:10 2014<br>
@@ -0,0 +1,41 @@<br>
+// RUN: %clang_cc1 -triple i686--windows -fms-compatibility -Oz -emit-llvm %s -o - | FileCheck %s<br>
+// RUN: %clang_cc1 -triple thumbv7--windows -fms-compatibility -Oz -emit-llvm %s -o - | FileCheck %s<br>
+<br>
+void *test_InterlockedExchangePointer(void * volatile *Target, void *Value) {<br>
+ return _InterlockedExchangePointer(Target, Value);<br>
+}<br>
+<br>
+// CHECK: define{{.*}}i8* @test_InterlockedExchangePointer(i8** %Target, i8* %Value){{.*}}{<br>
+// CHECK: entry:<br>
+// CHECK: %0 = bitcast i8** %Target to i32*<br>
+// CHECK: %1 = ptrtoint i8* %Value to i32<br>
+// CHECK: %2 = atomicrmw xchg i32* %0, i32 %1 seq_cst<br>
+// CHECK: %3 = inttoptr i32 %2 to i8*<br>
+// CHECK: ret i8* %3<br>
+// CHECK: }<br>
+<br>
+void *test_InterlockedCompareExchangePointer(void * volatile *Destination,<br>
+ void *Exchange, void *Comparand) {<br>
+ return _InterlockedCompareExchangePointer(Destination, Exchange, Comparand);<br>
+}<br>
+<br>
+// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer(i8** %Destination, i8* %Exchange, i8* %Comparand){{.*}}{<br>
+// CHECK: entry:<br>
+// CHECK: %0 = bitcast i8** %Destination to i32*<br>
+// CHECK: %1 = ptrtoint i8* %Exchange to i32<br>
+// CHECK: %2 = ptrtoint i8* %Comparand to i32<br>
+// CHECK: %3 = cmpxchg volatile i32* %0, i32 %2, i32 %1 seq_cst seq_cst<br>
+// CHECK: %4 = extractvalue { i32, i1 } %3, 0<br>
+// CHECK: %5 = inttoptr i32 %4 to i8*<br>
+// CHECK: ret i8* %5<br>
+// CHECK: }<br>
+<br>
+long test_InterlockedExchange(long *Target, long Value) {<br>
+ return _InterlockedExchange(Target, Value);<br>
+}<br>
+<br>
+// CHECK: define{{.*}}i32 @test_InterlockedExchange(i32* %Target, i32 %Value){{.*}}{<br>
+// CHECK: entry:<br>
+// CHECK: %0 = atomicrmw xchg i32* %Target, i32 %Value seq_cst<br>
+// CHECK: ret i32 %0<br>
+// CHECK: }<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>Saleem Abdulrasool<br>compnerd (at) compnerd (dot) org
</div></div>