r211216 - CodeGen: improve ms instrincics support

Saleem Abdulrasool compnerd at compnerd.org
Mon Jul 7 19:28:02 PDT 2014


On Fri, Jun 20, 2014 at 1:38 PM, Reid Kleckner <rnk at google.com> 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
:-(.  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.

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.

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 <compnerd at compnerd.org
> > 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>


-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140707/5c8fac64/attachment.html>


More information about the cfe-commits mailing list