Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clang's _bittest* MSVC intrinsics are incorrect for values outside [0, 31] #32535

Closed
rnk opened this issue May 26, 2017 · 2 comments
Closed

Clang's _bittest* MSVC intrinsics are incorrect for values outside [0, 31] #32535

rnk opened this issue May 26, 2017 · 2 comments
Labels
bugzilla Issues migrated from bugzilla clang:headers Headers provided by Clang, e.g. for intrinsics

Comments

@rnk
Copy link
Collaborator

rnk commented May 26, 2017

Bugzilla Link 33188
Resolution FIXED
Resolved on Jun 05, 2018 18:48
Version unspecified
OS Windows NT
CC @zmodem

Extended Description

I discovered this while staring at Microsoft's platform adaption layer code in ChakraCore: https://github.com/Microsoft/ChakraCore/blob/master/lib/Common/CommonPal.h#L647

__forceinline unsigned char _BitTestAndSet(LONG *_BitBase, int _BitPos)
{

#if defined(clang) && !defined(ARM)
// Clang doesn't expand _bittestandset intrinic to bts, and it's implemention also doesn't work for _BitPos >= 32
unsigned char retval = 0;
asm(
"bts %[_BitPos], %[_BitBase]\n\t"
"setc %b[retval]\n\t"
: [_BitBase] "+m" (*_BitBase), [retval] "+rm" (retval)
: [_BitPos] "ri" (_BitPos)
: "cc" // clobber condition code
);
return retval;
#else
return _bittestandset(_BitBase, _BitPos);
#endif
}

It's a shame that nobody filed this bug upstream. :(

The Intel manual supports confirms this view:
"""
BT—Bit Test
...
Selects the bit in a bit string (specified with the first operand, called the bit base) at the bit-position designated by
the bit offset (specified by the second operand) and stores the value of the bit in the CF flag. The bit base operand
can be a register or a memory location; the bit offset operand can be a register or an immediate value:
• If the bit base operand specifies a register, the instruction takes the modulo 16, 32, or 64 of the bit offset
operand (modulo size depends on the mode and register size; 64-bit operands are available only in 64-bit
mode).
• If the bit base operand specifies a memory location, the operand represents the address of the byte in memory
that contains the bit base (bit 0 of the specified byte) of the bit string. The range of the bit position that can be
referenced by the offset operand depends on the operand size.
See also: Bit(BitBase, BitOffset) on page 3-11.
"""

We either need to codegen this with an intrinsic or inline asm that will reliably select to bts, or we need to do an array indexing operation first.

@zmodem
Copy link
Collaborator

zmodem commented Aug 8, 2017

There was a patch for this, but it got stuck: https://reviews.llvm.org/D33616

@rnk
Copy link
Collaborator Author

rnk commented Jun 6, 2018

This should be fixed after r333978, r334059, and r334060.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:headers Headers provided by Clang, e.g. for intrinsics
Projects
None yet
Development

No branches or pull requests

2 participants