Re: r333978 - Reimplement the bittest intrinsic family as builtins with inline asm

2018-06-07 Thread Reid Kleckner via cfe-commits
This should be fixed now with r334239. In for a penny, in for a pound.

On Wed, Jun 6, 2018 at 6:09 PM Grang, Mandeep Singh 
wrote:

> @rnk I tried building spec2000/eon for Windows ARM64 and ran into these
> errors:
>
> use of undeclared identifier '_interlockedbittestandset_acq'
> use of undeclared identifier '_interlockedbittestandset_rel'
> use of undeclared identifier '_interlockedbittestandset_nf'
>
> I see that you have removed them in your patch. Apparently they are needed
> (at least for ARM64 Win).
>
> --Mandeep
>
> On 6/5/2018 11:01 AM, Reid Kleckner via cfe-commits wrote:
>
> On Tue, Jun 5, 2018 at 2:33 AM Martin Storsjö  wrote:
>
>> > // Many of MSVC builtins are on both x64 and ARM; to avoid repeating
>> code, we
>> > // handle them here.
>>
>> This doesn't seem thought through wrt non-x86 architectures. I'm not sure
>> if there's any similar suitable instruction to use on ARM/AArch64, but we
>> should at the very least fall back to doing whatever we did before this
>> change for anything not x86.
>>
>
>  I'll go back and take a look, but I'm not convinced that what we did
> before was correct for ARM either. I'm installing the Visual C++ aarch64
> compiler now so I can make sure we get it right.
>
>
> ___
> cfe-commits mailing 
> listcfe-comm...@lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r333978 - Reimplement the bittest intrinsic family as builtins with inline asm

2018-06-06 Thread Grang, Mandeep Singh via cfe-commits
@rnk I tried building spec2000/eon for Windows ARM64 and ran into these 
errors:


use of undeclared identifier '_interlockedbittestandset_acq'
use of undeclared identifier '_interlockedbittestandset_rel'
use of undeclared identifier '_interlockedbittestandset_nf'

I see that you have removed them in your patch. Apparently they are 
needed (at least for ARM64 Win).


--Mandeep

On 6/5/2018 11:01 AM, Reid Kleckner via cfe-commits wrote:
On Tue, Jun 5, 2018 at 2:33 AM Martin Storsjö > wrote:


> // Many of MSVC builtins are on both x64 and ARM; to avoid
repeating code, we
> // handle them here.

This doesn't seem thought through wrt non-x86 architectures. I'm
not sure
if there's any similar suitable instruction to use on ARM/AArch64,
but we
should at the very least fall back to doing whatever we did before
this
change for anything not x86.


 I'll go back and take a look, but I'm not convinced that what we did 
before was correct for ARM either. I'm installing the Visual C++ 
aarch64 compiler now so I can make sure we get it right.



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


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


Re: r333978 - Reimplement the bittest intrinsic family as builtins with inline asm

2018-06-05 Thread Martin Storsjö via cfe-commits

On Tue, 5 Jun 2018, Reid Kleckner via cfe-commits wrote:


Author: rnk
Date: Mon Jun  4 18:33:40 2018
New Revision: 333978

URL: http://llvm.org/viewvc/llvm-project?rev=333978=rev
Log:
Reimplement the bittest intrinsic family as builtins with inline asm

We need to implement _interlockedbittestandset as a builtin for
windows.h, so we might as well do the whole family. It reduces code
duplication anyway.

Fixes PR33188, a long standing bug in our bittest implementation
encountered by Chakra.

Added:
   cfe/trunk/test/CodeGen/bittest-intrin.c
Modified:
   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-other.c
   cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=333978=333977=333978=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Mon Jun  4 18:33:40 2018
@@ -744,6 +744,14 @@ BUILTIN(__builtin_rindex, "c*cC*i", "Fn"
LANGBUILTIN(_alloca,  "v*z", "n", ALL_MS_LANGUAGES)
LANGBUILTIN(__annotation, "wC*.","n", ALL_MS_LANGUAGES)
LANGBUILTIN(__assume, "vb",  "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittest,"UcNiC*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandcomplement,   "UcNi*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandreset,"UcNi*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandset,  "UcNi*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittest64,  "UcWiC*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandcomplement64, "UcWi*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandreset64,  "UcWi*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandset64,"UcWi*Wi", "n", ALL_MS_LANGUAGES)
LIBBUILTIN(_byteswap_ushort, "UsUs", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
LIBBUILTIN(_byteswap_ulong,  "UNiUNi",   "fnc", "stdlib.h", ALL_MS_LANGUAGES)
LIBBUILTIN(_byteswap_uint64, "ULLiULLi", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
@@ -783,7 +791,10 @@ LANGBUILTIN(_InterlockedOr,   "NiNiD*Ni"
LANGBUILTIN(_InterlockedXor8,  "ccD*c",   "n", ALL_MS_LANGUAGES)
LANGBUILTIN(_InterlockedXor16, "ssD*s",   "n", ALL_MS_LANGUAGES)
LANGBUILTIN(_InterlockedXor,   "NiNiD*Ni","n", ALL_MS_LANGUAGES)
-LANGBUILTIN(_interlockedbittestandset, "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandreset,   "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandreset64, "UcWiD*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset,   "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset64, "UcWiD*Wi", "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)

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=333978=333977=333978=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jun  4 18:33:40 2018
@@ -484,6 +484,37 @@ CodeGenFunction::emitBuiltinObjectSize(c
  return Builder.CreateCall(F, {Ptr, Min, NullIsUnknown});
}

+static RValue EmitBitTestIntrinsic(CodeGenFunction , const CallExpr *E,
+   char TestAnd, char Size,
+   bool Locked = false) {
+  Value *BitBase = CGF.EmitScalarExpr(E->getArg(0));
+  Value *BitPos = CGF.EmitScalarExpr(E->getArg(1));
+
+  // Build the assembly.
+  SmallString<64> Asm;
+  raw_svector_ostream AsmOS(Asm);
+  if (Locked)
+AsmOS << "lock ";
+  AsmOS << "bt";
+  if (TestAnd)
+AsmOS << TestAnd;
+  AsmOS << Size << " $2, ($1)\n\tsetc ${0:b}";
+
+  // Build the constraints. FIXME: We should support immediates when possible.
+  std::string Constraints = "=r,r,r,~{cc},~{flags},~{memory},~{fpsr}";
+  llvm::IntegerType *IntType = llvm::IntegerType::get(
+  CGF.getLLVMContext(),
+  CGF.getContext().getTypeSize(E->getArg(1)->getType()));
+  llvm::Type *IntPtrType = IntType->getPointerTo();
+  llvm::FunctionType *FTy =
+  llvm::FunctionType::get(CGF.Int8Ty, {IntPtrType, IntType}, false);
+
+  llvm::InlineAsm *IA =
+  llvm::InlineAsm::get(FTy, Asm, Constraints, /*SideEffects=*/true);
+  CallSite CS = CGF.Builder.CreateCall(IA, {BitBase, BitPos});
+  return RValue::get(CS.getInstruction());
+}
+
// Many of MSVC builtins are on both x64 and ARM; to avoid repeating code, we
// handle them here.


This doesn't seem thought through wrt non-x86 architectures. I'm not sure 
if there's any similar suitable instruction to use on ARM/AArch64, but we 
should at the very least fall back 

r333978 - Reimplement the bittest intrinsic family as builtins with inline asm

2018-06-04 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Jun  4 18:33:40 2018
New Revision: 333978

URL: http://llvm.org/viewvc/llvm-project?rev=333978=rev
Log:
Reimplement the bittest intrinsic family as builtins with inline asm

We need to implement _interlockedbittestandset as a builtin for
windows.h, so we might as well do the whole family. It reduces code
duplication anyway.

Fixes PR33188, a long standing bug in our bittest implementation
encountered by Chakra.

Added:
cfe/trunk/test/CodeGen/bittest-intrin.c
Modified:
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-other.c
cfe/trunk/test/CodeGen/ms-intrinsics.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=333978=333977=333978=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Mon Jun  4 18:33:40 2018
@@ -744,6 +744,14 @@ BUILTIN(__builtin_rindex, "c*cC*i", "Fn"
 LANGBUILTIN(_alloca,  "v*z", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__annotation, "wC*.","n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__assume, "vb",  "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittest,"UcNiC*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandcomplement,   "UcNi*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandreset,"UcNi*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandset,  "UcNi*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittest64,  "UcWiC*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandcomplement64, "UcWi*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandreset64,  "UcWi*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_bittestandset64,"UcWi*Wi", "n", ALL_MS_LANGUAGES)
 LIBBUILTIN(_byteswap_ushort, "UsUs", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
 LIBBUILTIN(_byteswap_ulong,  "UNiUNi",   "fnc", "stdlib.h", ALL_MS_LANGUAGES)
 LIBBUILTIN(_byteswap_uint64, "ULLiULLi", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
@@ -783,7 +791,10 @@ LANGBUILTIN(_InterlockedOr,   "NiNiD*Ni"
 LANGBUILTIN(_InterlockedXor8,  "ccD*c",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor16, "ssD*s",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor,   "NiNiD*Ni","n", ALL_MS_LANGUAGES)
-LANGBUILTIN(_interlockedbittestandset, "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandreset,   "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandreset64, "UcWiD*Wi", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset,   "UcNiD*Ni", "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(_interlockedbittestandset64, "UcWiD*Wi", "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)

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=333978=333977=333978=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jun  4 18:33:40 2018
@@ -484,6 +484,37 @@ CodeGenFunction::emitBuiltinObjectSize(c
   return Builder.CreateCall(F, {Ptr, Min, NullIsUnknown});
 }
 
+static RValue EmitBitTestIntrinsic(CodeGenFunction , const CallExpr *E,
+   char TestAnd, char Size,
+   bool Locked = false) {
+  Value *BitBase = CGF.EmitScalarExpr(E->getArg(0));
+  Value *BitPos = CGF.EmitScalarExpr(E->getArg(1));
+
+  // Build the assembly.
+  SmallString<64> Asm;
+  raw_svector_ostream AsmOS(Asm);
+  if (Locked)
+AsmOS << "lock ";
+  AsmOS << "bt";
+  if (TestAnd)
+AsmOS << TestAnd;
+  AsmOS << Size << " $2, ($1)\n\tsetc ${0:b}";
+
+  // Build the constraints. FIXME: We should support immediates when possible.
+  std::string Constraints = "=r,r,r,~{cc},~{flags},~{memory},~{fpsr}";
+  llvm::IntegerType *IntType = llvm::IntegerType::get(
+  CGF.getLLVMContext(),
+  CGF.getContext().getTypeSize(E->getArg(1)->getType()));
+  llvm::Type *IntPtrType = IntType->getPointerTo();
+  llvm::FunctionType *FTy =
+  llvm::FunctionType::get(CGF.Int8Ty, {IntPtrType, IntType}, false);
+
+  llvm::InlineAsm *IA =
+  llvm::InlineAsm::get(FTy, Asm, Constraints, /*SideEffects=*/true);
+  CallSite CS = CGF.Builder.CreateCall(IA, {BitBase, BitPos});
+  return RValue::get(CS.getInstruction());
+}
+
 // Many of MSVC builtins are on both x64 and ARM; to avoid repeating code, we
 // handle them here.
 enum class CodeGenFunction::MSVCIntrin {
@@ -497,7 +528,6 @@ enum class CodeGenFunction::MSVCIntrin {
   _InterlockedIncrement,
   _InterlockedOr,
   _InterlockedXor,
-  _interlockedbittestandset,
   __fastfail,
 };
 
@@