[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Hans Wennborg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299782: Implement _interlockedbittestandset as a builtin 
(authored by hans).

Changed prior to commit:
  https://reviews.llvm.org/D31736?vs=94433=94539#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31736

Files:
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/test/CodeGen/ms-intrinsics.c

Index: cfe/trunk/test/CodeGen/ms-intrinsics.c
===
--- cfe/trunk/test/CodeGen/ms-intrinsics.c
+++ cfe/trunk/test/CodeGen/ms-intrinsics.c
@@ -434,6 +434,17 @@
 
 #endif
 
+unsigned char test_interlockedbittestandset(volatile long *ptr, long bit) {
+  return _interlockedbittestandset(ptr, bit);
+}
+// CHECK-LABEL: define{{.*}} i8 @test_interlockedbittestandset
+// CHECK: %0 = shl i32 1, %bit
+// CHECK: %1 = atomicrmw or i32* %ptr, i32 %0 seq_cst
+// CHECK: %2 = lshr i32 %1, %bit
+// CHECK: %3 = trunc i32 %2 to i8
+// CHECK: %4 = and i8 %3, 1
+// CHECK: ret i8 %4
+
 void test__fastfail() {
   __fastfail(42);
 }
Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -173,7 +173,6 @@
 void __cdecl _enable(void);
 long _InterlockedAddLargeStatistic(__int64 volatile *_Addend, long _Value);
 unsigned char _interlockedbittestandreset(long volatile *, long);
-static __inline__
 unsigned char _interlockedbittestandset(long volatile *, long);
 long _InterlockedCompareExchange_HLEAcquire(long volatile *, long, long);
 long _InterlockedCompareExchange_HLERelease(long volatile *, long, long);
@@ -369,11 +368,6 @@
   *_BitBase = *_BitBase | (1 << _BitPos);
   return _Res;
 }
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_interlockedbittestandset(long volatile *_BitBase, long _BitPos) {
-  long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_SEQ_CST);
-  return (_PrevVal >> _BitPos) & 1;
-}
 #if defined(__arm__) || defined(__aarch64__)
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -492,6 +492,7 @@
   _InterlockedIncrement,
   _InterlockedOr,
   _InterlockedXor,
+  _interlockedbittestandset,
   __fastfail,
 };
 
@@ -559,6 +560,22 @@
   case MSVCIntrin::_InterlockedXor:
 return MakeBinaryAtomicValue(*this, AtomicRMWInst::Xor, E);
 
+  case MSVCIntrin::_interlockedbittestandset: {
+llvm::Value *Addr = EmitScalarExpr(E->getArg(0));
+llvm::Value *Bit = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+AtomicRMWInst::Or, Addr,
+Builder.CreateShl(ConstantInt::get(Bit->getType(), 1), Bit),
+llvm::AtomicOrdering::SequentiallyConsistent);
+// Shift the relevant bit to the least significant position, truncate to
+// the result type, and test the low bit.
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =
+Builder.CreateTrunc(Shifted, ConvertType(E->getType()));
+return Builder.CreateAnd(Truncated,
+ ConstantInt::get(Truncated->getType(), 1));
+  }
+
   case MSVCIntrin::_InterlockedDecrement: {
 llvm::Type *IntTy = ConvertType(E->getType());
 AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
@@ -2238,6 +2255,9 @@
   case Builtin::BI_InterlockedXor16:
   case Builtin::BI_InterlockedXor:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E));
+  case Builtin::BI_interlockedbittestandset:
+return RValue::get(
+EmitMSVCBuiltinExpr(MSVCIntrin::_interlockedbittestandset, E));
 
   case Builtin::BI__exception_code:
   case Builtin::BI_exception_code:
@@ -2309,10 +2329,8 @@
 break;
   }
 
-  case Builtin::BI__fastfail: {
+  case Builtin::BI__fastfail:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::__fastfail, E));
-break;
-  }
 
   case Builtin::BI__builtin_coro_size: {
 auto & Context = getContext();
Index: cfe/trunk/include/clang/Basic/Builtins.def
===
--- cfe/trunk/include/clang/Basic/Builtins.def
+++ cfe/trunk/include/clang/Basic/Builtins.def
@@ -756,6 +756,7 @@
 LANGBUILTIN(_InterlockedXor8,  "ccD*c",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor16, "ssD*s",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor,   "LiLiD*Li","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset, "UcLiD*Li", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__popcnt16, "UsUs", "nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__popcnt,   "UiUi", "nc", ALL_MS_LANGUAGES)

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm




Comment at: lib/CodeGen/CGBuiltin.cpp:570
+llvm::AtomicOrdering::SequentiallyConsistent);
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =

hans wrote:
> rnk wrote:
> > Can you comment that this shifts the relevant bit into the low bit, 
> > truncates to i8, and then tests the low bit? At first I was thinking "of 
> > course, we just do AND %old, $constant to test the bit".
> > 
> > Do we successfully pattern match this idiom to `lock bts`? Is there a 
> > pattern match we should target?
> Added the comment.
> 
> No, we generate horrible code for this, but it's the same as we did for the 
> inline version in intrin.h. We have BTS instructions in the .td file, but no 
> patterns for them.
> 
> I thought a bit about adding a pattern match for this, but I'm not sure it 
> would be worth the effort.
Let's file a bug for it and call it a day.


https://reviews.llvm.org/D31736



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:570
+llvm::AtomicOrdering::SequentiallyConsistent);
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =

rnk wrote:
> Can you comment that this shifts the relevant bit into the low bit, truncates 
> to i8, and then tests the low bit? At first I was thinking "of course, we 
> just do AND %old, $constant to test the bit".
> 
> Do we successfully pattern match this idiom to `lock bts`? Is there a pattern 
> match we should target?
Added the comment.

No, we generate horrible code for this, but it's the same as we did for the 
inline version in intrin.h. We have BTS instructions in the .td file, but no 
patterns for them.

I thought a bit about adding a pattern match for this, but I'm not sure it 
would be worth the effort.


https://reviews.llvm.org/D31736



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Hans Wennborg via Phabricator via cfe-commits]

hans updated this revision to Diff 94433.
hans added a comment.

Add comment.


https://reviews.llvm.org/D31736

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics.c

Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -434,6 +434,17 @@
 
 #endif
 
+unsigned char test_interlockedbittestandset(volatile long *ptr, long bit) {
+  return _interlockedbittestandset(ptr, bit);
+}
+// CHECK-LABEL: define{{.*}} i8 @test_interlockedbittestandset
+// CHECK: %0 = shl i32 1, %bit
+// CHECK: %1 = atomicrmw or i32* %ptr, i32 %0 seq_cst
+// CHECK: %2 = lshr i32 %1, %bit
+// CHECK: %3 = trunc i32 %2 to i8
+// CHECK: %4 = and i8 %3, 1
+// CHECK: ret i8 %4
+
 void test__fastfail() {
   __fastfail(42);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -173,7 +173,6 @@
 void __cdecl _enable(void);
 long _InterlockedAddLargeStatistic(__int64 volatile *_Addend, long _Value);
 unsigned char _interlockedbittestandreset(long volatile *, long);
-static __inline__
 unsigned char _interlockedbittestandset(long volatile *, long);
 long _InterlockedCompareExchange_HLEAcquire(long volatile *, long, long);
 long _InterlockedCompareExchange_HLERelease(long volatile *, long, long);
@@ -369,11 +368,6 @@
   *_BitBase = *_BitBase | (1 << _BitPos);
   return _Res;
 }
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_interlockedbittestandset(long volatile *_BitBase, long _BitPos) {
-  long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_SEQ_CST);
-  return (_PrevVal >> _BitPos) & 1;
-}
 #if defined(__arm__) || defined(__aarch64__)
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -492,6 +492,7 @@
   _InterlockedIncrement,
   _InterlockedOr,
   _InterlockedXor,
+  _interlockedbittestandset,
   __fastfail,
 };
 
@@ -559,6 +560,22 @@
   case MSVCIntrin::_InterlockedXor:
 return MakeBinaryAtomicValue(*this, AtomicRMWInst::Xor, E);
 
+  case MSVCIntrin::_interlockedbittestandset: {
+llvm::Value *Addr = EmitScalarExpr(E->getArg(0));
+llvm::Value *Bit = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+AtomicRMWInst::Or, Addr,
+Builder.CreateShl(ConstantInt::get(Bit->getType(), 1), Bit),
+llvm::AtomicOrdering::SequentiallyConsistent);
+// Shift the relevant bit to the least significant position, truncate to
+// the result type, and test the low bit.
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =
+Builder.CreateTrunc(Shifted, ConvertType(E->getType()));
+return Builder.CreateAnd(Truncated,
+ ConstantInt::get(Truncated->getType(), 1));
+  }
+
   case MSVCIntrin::_InterlockedDecrement: {
 llvm::Type *IntTy = ConvertType(E->getType());
 AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
@@ -2238,6 +2255,9 @@
   case Builtin::BI_InterlockedXor16:
   case Builtin::BI_InterlockedXor:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E));
+  case Builtin::BI_interlockedbittestandset:
+return RValue::get(
+EmitMSVCBuiltinExpr(MSVCIntrin::_interlockedbittestandset, E));
 
   case Builtin::BI__exception_code:
   case Builtin::BI_exception_code:
@@ -2309,10 +2329,8 @@
 break;
   }
 
-  case Builtin::BI__fastfail: {
+  case Builtin::BI__fastfail:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::__fastfail, E));
-break;
-  }
 
   case Builtin::BI__builtin_coro_size: {
 auto & Context = getContext();
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -756,6 +756,7 @@
 LANGBUILTIN(_InterlockedXor8,  "ccD*c",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor16, "ssD*s",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor,   "LiLiD*Li","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset, "UcLiD*Li", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__popcnt16, "UsUs", "nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__popcnt,   "UiUi", "nc", ALL_MS_LANGUAGES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lib/CodeGen/CGBuiltin.cpp:570
+llvm::AtomicOrdering::SequentiallyConsistent);
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =

Can you comment that this shifts the relevant bit into the low bit, truncates 
to i8, and then tests the low bit? At first I was thinking "of course, we just 
do AND %old, $constant to test the bit".

Do we successfully pattern match this idiom to `lock bts`? Is there a pattern 
match we should target?


https://reviews.llvm.org/D31736



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31736: Implement _interlockedbittestandset as a builtin

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.

It's used by MS headers in VS 2017 without including intrin.h, so we can't 
implement it in the header anymore.


https://reviews.llvm.org/D31736

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/ms-intrinsics.c

Index: test/CodeGen/ms-intrinsics.c
===
--- test/CodeGen/ms-intrinsics.c
+++ test/CodeGen/ms-intrinsics.c
@@ -434,6 +434,17 @@
 
 #endif
 
+unsigned char test_interlockedbittestandset(volatile long *ptr, long bit) {
+  return _interlockedbittestandset(ptr, bit);
+}
+// CHECK-LABEL: define{{.*}} i8 @test_interlockedbittestandset
+// CHECK: %0 = shl i32 1, %bit
+// CHECK: %1 = atomicrmw or i32* %ptr, i32 %0 seq_cst
+// CHECK: %2 = lshr i32 %1, %bit
+// CHECK: %3 = trunc i32 %2 to i8
+// CHECK: %4 = and i8 %3, 1
+// CHECK: ret i8 %4
+
 void test__fastfail() {
   __fastfail(42);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -173,7 +173,6 @@
 void __cdecl _enable(void);
 long _InterlockedAddLargeStatistic(__int64 volatile *_Addend, long _Value);
 unsigned char _interlockedbittestandreset(long volatile *, long);
-static __inline__
 unsigned char _interlockedbittestandset(long volatile *, long);
 long _InterlockedCompareExchange_HLEAcquire(long volatile *, long, long);
 long _InterlockedCompareExchange_HLERelease(long volatile *, long, long);
@@ -369,11 +368,6 @@
   *_BitBase = *_BitBase | (1 << _BitPos);
   return _Res;
 }
-static __inline__ unsigned char __DEFAULT_FN_ATTRS
-_interlockedbittestandset(long volatile *_BitBase, long _BitPos) {
-  long _PrevVal = __atomic_fetch_or(_BitBase, 1l << _BitPos, __ATOMIC_SEQ_CST);
-  return (_PrevVal >> _BitPos) & 1;
-}
 #if defined(__arm__) || defined(__aarch64__)
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _interlockedbittestandset_acq(long volatile *_BitBase, long _BitPos) {
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -492,6 +492,7 @@
   _InterlockedIncrement,
   _InterlockedOr,
   _InterlockedXor,
+  _interlockedbittestandset,
   __fastfail,
 };
 
@@ -559,6 +560,20 @@
   case MSVCIntrin::_InterlockedXor:
 return MakeBinaryAtomicValue(*this, AtomicRMWInst::Xor, E);
 
+  case MSVCIntrin::_interlockedbittestandset: {
+llvm::Value *Addr = EmitScalarExpr(E->getArg(0));
+llvm::Value *Bit = EmitScalarExpr(E->getArg(1));
+AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+AtomicRMWInst::Or, Addr,
+Builder.CreateShl(ConstantInt::get(Bit->getType(), 1), Bit),
+llvm::AtomicOrdering::SequentiallyConsistent);
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =
+Builder.CreateTrunc(Shifted, ConvertType(E->getType()));
+return Builder.CreateAnd(Truncated,
+ ConstantInt::get(Truncated->getType(), 1));
+  }
+
   case MSVCIntrin::_InterlockedDecrement: {
 llvm::Type *IntTy = ConvertType(E->getType());
 AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
@@ -2238,6 +2253,9 @@
   case Builtin::BI_InterlockedXor16:
   case Builtin::BI_InterlockedXor:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E));
+  case Builtin::BI_interlockedbittestandset:
+return RValue::get(
+EmitMSVCBuiltinExpr(MSVCIntrin::_interlockedbittestandset, E));
 
   case Builtin::BI__exception_code:
   case Builtin::BI_exception_code:
@@ -2309,10 +2327,8 @@
 break;
   }
 
-  case Builtin::BI__fastfail: {
+  case Builtin::BI__fastfail:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::__fastfail, E));
-break;
-  }
 
   case Builtin::BI__builtin_coro_size: {
 auto & Context = getContext();
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -756,6 +756,7 @@
 LANGBUILTIN(_InterlockedXor8,  "ccD*c",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor16, "ssD*s",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor,   "LiLiD*Li","n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset, "UcLiD*Li", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__popcnt16, "UsUs", "nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__popcnt,   "UiUi", "nc", ALL_MS_LANGUAGES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits