-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Use BitOperations::PopCount() in genCountBits() #83661
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
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsNoticed that we use slow way of calculating bits set in a mask instead of
|
|
Running base: 4.97 secs @tannergooding - what do you think? |
|
I expect it will be more performant on average. Were you able to get a profile via Intel VTune or AMD uProf which could help highlight the execution time percentage change for functions using this API? |
| */ | ||
|
|
||
| return cnt; | ||
| inline unsigned genCountBits(uint32_t bits) |
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.
Any reason to not just use BitOperations::PopCount directly?
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.
I was getting some compilation errors but didn't explore other options. I will double check before merging.



Noticed that we use slow way of calculating bits set in a mask instead of
PopCount. Unix TP should improve with this, but need to see how Windows react, because in windows, we don't have popcount and do bit twiddling that involves multiplication. This might prove slightly expensive for inputs that has smaller number of bits set.