Avoid __popcnt and __popcnt64 intrinsics for MSC#2944
Conversation
|
Some of intrinsics like |
| // otherwise, see if adding one turns this into a 1-bit thing, 00011111 + 1 | ||
| // => 00100000 | ||
| if (PopCount(mask + 1) != 1) { | ||
| if (mask & (mask + 1)) { |
There was a problem hiding this comment.
(mask & (mask + 1)) != 0 is not fully equivalent to PopCount(mask + 1) != 1 because not handle case when mask == uint32_t(-1) but this already checked earlier so we can use more simplify version without this special case here
There was a problem hiding this comment.
Makes sense to me. It would be good to fix the comment to reflect the new calculation, since it currently suggests that we are using popcount.
tlively
left a comment
There was a problem hiding this comment.
Even if this doesn't fix the original issue, this still seems like a good change if __popcnt is not guaranteed to work for all CPUs on windows.
|
Sounds good, let's merge this then. @RReverser if you get a chance, it would still be good to confirm if this fixes that issue. |
|
@kripken I already mentioned on the original issue that I tried it and it didn't change anything for me (neither size nor the reproduction): #2942 (comment) |
|
Got it, thanks @RReverser |
Potentially fix #2942Unfortunately not but still relevant for other environments