Update PAL and GC BitScanReverse to use __builtin_clz
#89350
Conversation
The existing PAL code was using `__builtin_clzl` which is intended for platforms where `long` is 64 bits. Instead use `__builtin_clz`. The GC version had a similar issue so I've changed that too. The JIT version was already using `__builtin_clz`. Fixes dotnet#89340.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThe existing PAL code was using The GC version had a similar issue so I've changed that too. The JIT version was already using Fixes #89340.
|
|
@jakobbotsch PTAL cc @dotnet/jit-contrib |
| @@ -300,12 +300,12 @@ inline uint8_t BitScanReverse(uint32_t *bitIndex, uint32_t mask) | |||
| #ifdef _MSC_VER | |||
| return _BitScanReverse((unsigned long*)bitIndex, mask); | |||
There was a problem hiding this comment.
nit: replace unsigned long with uint32_t or just unsigned to remove confusion?
|
Surprised it never caused any issues until now 🤔 |
|
The title of the PR says "BitScanForward" but you're actually changing "BitScanReverse" |
AndyAyersMS
changed the title
BitScanForward to use __builtin_clzBitScanReverse to use __builtin_clz
Indeed. Thanks. |
|
Errors are known. |
ghost
locked as resolved and limited conversation to collaborators
The existing PAL code was using
__builtin_clzlwhich is intended for platforms wherelongis 64 bits. Instead use__builtin_clz.The GC version had a similar issue so I've changed that too. The JIT version was already using
__builtin_clz.Fixes #89340.