[PATCH] D33616: [MS] Fix _bittest* intrinsics for values bigger than 31

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Darn, I think Richard is right, the signed div/mod doesn't do the right thing for negative values. Honestly I need to rig up a test case for this, and then I'll come back to it.

What do you folks think is best:

- Add an LLVM intrinsic for this and use it
- Use inline assembly in intrin.h
- Emit an InlineAsm call directly in CGBuiltin.cpp
- Keep expanding this to plain C and wait for us to eventually pattern match this in LLVM CodeGen

The LLVM pattern match optimization feels increasingly difficult because of all the edge cases around negative indices, and that's pushing me back towards doing this as an instruction.



================
Comment at: clang/lib/Headers/intrin.h:345-347
+  _BitBase += (_BitPos / 32);
+  _BitPos %= 32;
   return (*_BitBase >> _BitPos) & 1;
----------------
majnemer wrote:
> `_bittest` seems to expand to `(((unsigned char const *)_BitBase)[_BitPos  >> 3] >> (_BitPos & 7)) & 1` on CL ARM: https://godbolt.org/g/Yc8rMH
> 
> Perhaps these are the intended semantics?
Personally I think we need to match the x86 behavior on x86.


https://reviews.llvm.org/D33616