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

Improved halide_popcount #7225

Merged
merged 3 commits into from
Jan 25, 2023
Merged

Improved halide_popcount #7225

merged 3 commits into from
Jan 25, 2023

Conversation

Aelphy
Copy link
Contributor

@Aelphy Aelphy commented Dec 9, 2022

This is a slightly more efficient version of a popcount, which has as many iterations as many ones are in the binary representation.

@@ -155,8 +155,9 @@ template<typename T>
inline int halide_popcount(T a) {
int bits_set = 0;
while (a != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, ~all modern compilers should have popcount as an intrinsic -- see popcount64 in Util.cpp, maybe we should just adapt that here

@steven-johnson
Copy link
Contributor

What's the story on this, ready to land?

@@ -152,15 +152,39 @@ inline float float_from_bits(uint32_t bits) {
}

template<typename T>
inline int halide_popcount(T a) {
inline int halide_popcount_fallback(T a) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave a link to the source for this algorithm for the reference (found this one https://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetKernighan, for example).

bits_set += a & 1;
a >>= 1;
bits_set += 1;
// NOTE(aelphy): remove least significant zeros and the first met one
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just make it a regular comment (i.e. remove NOTE(aelphy) part).

@steven-johnson
Copy link
Contributor

Where does this PR stand?

@vksnk vksnk merged commit dd973f4 into halide:main Jan 25, 2023
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Improved halide_popcount

* reused popcount64 from Utils.cpp in CodeGen_C

* Fixed comment for popcount
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants