-
Notifications
You must be signed in to change notification settings - Fork 290
Replace LZCNT with BSR instruction in hdr_histogram #230
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
Conversation
The LZCNT instruction produced by __lzcnt on Windows is not available on many CPUs in use today. BSR is a safe replacement requiring only minimal additional work.
Hi @SeverinLeonhardt, thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. Sincerely, |
Thank you @SeverinLeonhardt for signing the Contribution License Agreement. Cheers, |
@SeverinLeonhardt Thanks for researching and getting a pull request together. I just had one issue compiling on Windows, otherwise, LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be const because it's reused for the next BSR.
I noticed another issue too that's unrelated to the changes you made. For GCC-based compilers |
__builtin_clzll is undefined for 0 so this case must be handled separately.
Updated the pull request as you've suggested. |
Thanks for updating. Looks good. |
Replace LZCNT with BSR instruction in hdr_histogram
The LZCNT instruction produced by __lzcnt on Windows is not available
on many CPUs in use today. BSR is a safe replacement requiring only
minimal additional work.