2014-07-08 6:28 GMT+04:00 Saleem Abdulrasool <[email protected]>: > On Fri, Jun 20, 2014 at 1:38 PM, Reid Kleckner <[email protected]> wrote: >> >> 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. >> >> This is the problematic code: >> inline static void *_InterlockedCompareExchangePointer( >> void *volatile *Destination, >> void *Exchange, void *Comparand) { >> return reinterpret_cast<void*>( >> _InterlockedCompareExchange( >> reinterpret_cast<long volatile*>(Destination), // NOLINT >> reinterpret_cast<long>(Exchange), // NOLINT >> reinterpret_cast<long>(Comparand))); // NOLINT >> } > > > 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: > > I seem to be having issues building check-asan, as in missing the target > :-(.
You should probably check out compiler to projects/ > However, since you were kind enough to provide the reduction of the > issue, it really helps. > > Im conflicted on the best approach to solve this. We could either > a) filter this in FunctionDecl::getBuiltinID > b) not provide this in ASAN. I'm deferring the choice to Reid as I'm not up-to-date with the builtins progress. > 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. This is correct. > 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. > > 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. > >> >> >> On Wed, Jun 18, 2014 at 1:51 PM, Saleem Abdulrasool >> <[email protected]> wrote: >>> >>> Author: compnerd >>> Date: Wed Jun 18 15:51:10 2014 >>> New Revision: 211216 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=211216&view=rev >>> Log: >>> CodeGen: improve ms instrincics support >>> >>> 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. >>> >>> Added: >>> cfe/trunk/test/CodeGen/ms-intrinsics.c >>> Modified: >>> cfe/trunk/include/clang/Basic/Builtins.def >>> cfe/trunk/lib/CodeGen/CGBuiltin.cpp >>> cfe/trunk/lib/Headers/Intrin.h >>> >>> Modified: cfe/trunk/include/clang/Basic/Builtins.def >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=211216&r1=211215&r2=211216&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/Builtins.def (original) >>> +++ cfe/trunk/include/clang/Basic/Builtins.def Wed Jun 18 15:51:10 2014 >>> @@ -683,9 +683,12 @@ LANGBUILTIN(__noop, "v.", "n", AL >>> 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 >>> >>> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=211216&r1=211215&r2=211216&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Jun 18 15:51:10 2014 >>> @@ -1516,6 +1516,35 @@ RValue CodeGenFunction::EmitBuiltinExpr( >>> 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, IntType); >>> + >>> + llvm::Value *Comparand = >>> + Builder.CreatePtrToInt(EmitScalarExpr(E->getArg(2)), IntType); >>> + >>> + auto Result = Builder.CreateAtomicCmpXchg(Destination, Comparand, >>> Exchange, >>> + SequentiallyConsistent, >>> + SequentiallyConsistent); >>> + Result->setVolatile(true); >>> + >>> + return >>> RValue::get(Builder.CreateIntToPtr(Builder.CreateExtractValue(Result, >>> + >>> 0), >>> + RTy)); >>> + } >>> case Builtin::BI_InterlockedCompareExchange: { >>> AtomicCmpXchgInst *CXI = Builder.CreateAtomicCmpXchg( >>> EmitScalarExpr(E->getArg(0)), >>> >>> Modified: cfe/trunk/lib/Headers/Intrin.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/Intrin.h?rev=211216&r1=211215&r2=211216&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/Headers/Intrin.h (original) >>> +++ cfe/trunk/lib/Headers/Intrin.h Wed Jun 18 15:51:10 2014 >>> @@ -223,8 +223,7 @@ 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__ >>> @@ -411,7 +410,6 @@ __int64 _InterlockedCompareExchange64_HL >>> __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, >>> @@ -422,7 +420,6 @@ 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); >>> @@ -785,22 +782,12 @@ _InterlockedExchange16(short volatile *_ >>> __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 >>> @@ -817,14 +804,6 @@ _InterlockedCompareExchange16(short vola >>> __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) { >>> >>> Added: cfe/trunk/test/CodeGen/ms-intrinsics.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-intrinsics.c?rev=211216&view=auto >>> >>> ============================================================================== >>> --- cfe/trunk/test/CodeGen/ms-intrinsics.c (added) >>> +++ cfe/trunk/test/CodeGen/ms-intrinsics.c Wed Jun 18 15:51:10 2014 >>> @@ -0,0 +1,41 @@ >>> +// 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 >>> *Comparand) { >>> + return _InterlockedCompareExchangePointer(Destination, Exchange, >>> Comparand); >>> +} >>> + >>> +// CHECK: define{{.*}}i8* @test_InterlockedCompareExchangePointer(i8** >>> %Destination, i8* %Exchange, i8* %Comparand){{.*}}{ >>> +// CHECK: entry: >>> +// CHECK: %0 = bitcast i8** %Destination to i32* >>> +// CHECK: %1 = ptrtoint i8* %Exchange to i32 >>> +// CHECK: %2 = ptrtoint i8* %Comparand to i32 >>> +// CHECK: %3 = cmpxchg volatile i32* %0, i32 %2, i32 %1 seq_cst >>> seq_cst >>> +// CHECK: %4 = extractvalue { i32, i1 } %3, 0 >>> +// CHECK: %5 = inttoptr i32 %4 to i8* >>> +// CHECK: ret i8* %5 >>> +// 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: } >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> > > > > -- > Saleem Abdulrasool > compnerd (at) compnerd (dot) org _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
