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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

My feeling is that there's not much value in adding an LLVM intrinsic for something that can be expressed directly in a handful of IR instructions; those instructions seem like the way to express this that would allow the most optimization potential, and the backend should be able to pattern match this to a BT instruction. Based on that, I'm reluctant to add clang builtins for this, partly because it looks like we'd need quite a lot of them and partly because (in the absence of an IR intrinsic) they'd just get lowered into the same instructions as the plain C code anyway.



================
Comment at: clang/lib/Headers/intrin.h:398
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _bittest64(__int64 const *_BitBase, __int64 _BitPos) {
+  _BitBase += (_BitPos / 64);
----------------
I'm confused: why are there distinct ...32 and ...64 versions of these, if the bit number can leave the (d|q)word anyway? Are we supposed to number bits right-to-left within words on big endian targets? Or is this guaranteeing something about alignment or about being able to load a full 32/64 bits at the requested word offset?


https://reviews.llvm.org/D33616