include: util: Add generic function to count bits set in a value #87943

Thalley · 2025-04-01T07:24:01Z

Adds a generic function that will count the number of bits set in a value.

It uses POPCOUNT (e.g. __builtin_popcount for GCC) if available,
or else it will use Brian Kernighan’s Algorithm to count bits.

POPCOUNT will likely always support unsigned ints, but the function
was implemented to use it with uint8_t for the sake of simplicity
and compatibility with Brian Kernighan’s Algorithm.

A generic solution was chosen rather than a macro/function per
type (e.g. uint8_t, uint16_t, etc.) as that is easier to maintain
and also supports array types (e.g. counting the number of bits
in 128 or 256 octet arrays).

fixes #87718

jfischer-no · 2025-04-01T08:27:15Z

include/zephyr/sys/util.h

+ * @param value The value to count number of bits set of
+ * @param len The number of octets in @p value
+ */
+static inline size_t zephyr_count_bits(const void *value, size_t len)


Tough, this could be just sys_count_bits() use the well-known sys_ prefix and be aligned with existing utils, for unknown reasons some people prefer to use the zephyr_ prefix. But zephyr_ is not really better, because it is also used by many other software projects, and it does not minimize the likelihood of collisions. Instead, we should use UUID version 5 as a prefix, like 7f756fa7_4e84_529e_b406_276069b9bcd0_.

Suggested change

static inline size_t zephyr_count_bits(const void *value, size_t len)

static inline size_t 7f756fa7_4e84_529e_b406_276069b9bcd0_count_bits(const void *const value, const size_t len)

{

I get your point, and I don't really have a strong preference myself. I went with zephyr_ as that seemed like the least controversial output from #64627

but zephyr_ is not really better, because it is also used by many other software projects

Can you share some of these projects so we can consider them properly?

Instead, we should use UUID version 5 as a prefix, like 7f756fa7_4e84_529e_b406_276069b9bcd0_.

The C language prohibits starting identifiers with a number 🫣

We can use 30a23962-5d6b-4c76-87aa-cc799d50a6a7 and with "Zephyr RTOS" as namespace, which results in c496e475_323e_5ca0_8fa1_39f91557c0e0.

Can you share some of these projects so we can consider them properly?

There are some cloud and cryptobro projects. Just search for zephyr.

Copilot

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

doc/releases/release-notes-4.2.rst: Language not supported

include/zephyr/sys/util.h

Copilot

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Files not reviewed (1)

doc/releases/release-notes-4.2.rst: Language not supported

Thalley · 2025-04-04T20:49:22Z

include/zephyr/sys/util.h

+#ifdef POPCOUNT
+	for (; i < len / sizeof(unsigned int); i++) {
+		unsigned int val = ((unsigned int *)value)[i];
+
+		cnt += POPCOUNT(val);
+	}
+	i *= sizeof(unsigned int); /* convert to a uint8_t index for the remainder (if any) */
+#endif


POPCOUNT works very well for unsigned int types for both GCC and Clang. There are additional POPCOUNT functions for other sizes, but not sure if they provide any meaningful optimizations.

It is, however (from my benchmark tests) very important to provide a unsigned int to GCC; there was a factor of more than 10000 in terms of speed on whether I provided a unsigned int or a uint8_t to POPCOUNT, so this version is the fastest I've been able to produce which handles any size, but being extremely fast if POPCOUNT has been defined.

Copilot

Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.

Files not reviewed (1)

doc/releases/release-notes-4.2.rst: Language not supported

Thalley · 2025-04-22T08:40:43Z

Rebased to fix merge conflicts

Thalley · 2025-05-01T07:22:48Z

Also worth mentioning that C23 comes with a stdc_count_ones* set of functions for this purpose too, which seems to be an alternative to POPCOUNT at least

pdgendt · 2025-05-05T11:20:20Z

Based on the Arch meeting discussions, and the past vote to keep "area" prefixes, I think sys_count_bits would be preferred now?

Thalley · 2025-05-05T12:20:50Z

Based on the Arch meeting discussions, and the past vote to keep "area" prefixes, I think sys_count_bits would be preferred now?

Maybe - The last minutes didn't specify any conclusion, and I'd prefer waiting to update this PR until we reach a consensus to avoid having to update this PR multiple times

Adds a generic function that will count the number of bits set in a value. It uses POPCOUNT (e.g. __builtin_popcount for GCC) if available, or else it will use Brian Kernighan’s Algorithm to count bits. POPCOUNT will likely always support unsigned ints, but the function was implemented to use it with uint8_t for the sake of simplicity and compatibility with Brian Kernighan’s Algorithm. A generic solution was chosen rather than a macro/function per type (e.g. uint8_t, uint16_t, etc.) as that is easier to maintain and also supports array types (e.g. counting the number of bits in 128 or 256 octet arrays). Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>

Instead of re-implementing or assuming that POPCOUNT is available we now use the generic function from Zephyr. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>

Adopt new zephyr_count_bits helper from util.h and avoid having conflicting definition Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>

andyross

Hate to be That Guy in a patch that has clearly been through the bike shed too many times already, but there're a ton of whitespace-only changes in that first patch obscuring what is actually a very simple semantic change. Can you split those out?

sonarqubecloud · 2025-07-01T12:46:35Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

This was referenced Apr 1, 2025

Long term plan to unify public zephyr APIs using ONE prefix #64627

Open

Update POPCOUNT macro to be type-flexible #87718

Open

jfischer-no requested changes Apr 1, 2025

View reviewed changes

Thalley marked this pull request as ready for review April 1, 2025 12:11

github-actions bot added area: Testsuite Testsuite Release Notes To be mentioned in the release notes platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) area: ADC Analog-to-Digital Converter (ADC) area: Bluetooth Audio labels Apr 1, 2025

github-project-automation bot added this to Bluetooth LE Audio Apr 1, 2025

github-actions bot added the area: Bluetooth label Apr 1, 2025

github-project-automation bot moved this to In Review in Bluetooth LE Audio Apr 1, 2025

github-actions bot added area: Kernel area: Samples Samples area: Base OS Base OS Library (lib/os) labels Apr 1, 2025

github-actions bot requested review from asbjornsabo, attie-argentum, babrsn, Casper-Bonde-Bose, fabiobaltieri, jhedberg, kartben, MariuszSkamra, mnkp, nandojve, pin-zephyr and sjanc April 1, 2025 12:13

github-actions bot assigned fabiobaltieri and kartben Apr 1, 2025

fabiobaltieri assigned nashif Apr 4, 2025

fabiobaltieri assigned andyross and unassigned kartben and fabiobaltieri Apr 4, 2025

Thalley requested a review from Copilot April 4, 2025 18:30

Copilot AI reviewed Apr 4, 2025

View reviewed changes

include/zephyr/sys/util.h Outdated Show resolved Hide resolved

Thalley force-pushed the zephyr_count_bits branch from a052223 to b659ec7 Compare April 4, 2025 18:51

Thalley requested a review from Copilot April 4, 2025 18:51

Copilot AI reviewed Apr 4, 2025

View reviewed changes

Thalley force-pushed the zephyr_count_bits branch from b659ec7 to 70ca373 Compare April 4, 2025 20:46

Thalley commented Apr 4, 2025

View reviewed changes

Thalley requested a review from Copilot April 4, 2025 20:50

Copilot AI reviewed Apr 4, 2025

View reviewed changes

Thalley force-pushed the zephyr_count_bits branch from 70ca373 to 70e017c Compare April 22, 2025 08:40

Thalley force-pushed the zephyr_count_bits branch from 70e017c to 070c3c7 Compare May 5, 2025 11:02

Thalley force-pushed the zephyr_count_bits branch 3 times, most recently from 93a2909 to 87cac90 Compare May 19, 2025 09:21

Thalley force-pushed the zephyr_count_bits branch from 87cac90 to 3f94320 Compare June 16, 2025 11:36

Thalley and others added 4 commits July 1, 2025 14:20

Bluetooth: Audio: Use generic zephyr_count_bits to count bits

d3a287a

Instead of re-implementing or assuming that POPCOUNT is available we now use the generic function from Zephyr. Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>

drivers: adc: adopt new zephyr_count_bits util function

b09f01b

Adopt new zephyr_count_bits helper from util.h and avoid having conflicting definition Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>

tests: kernel: adopt new zephyr_count_bits util function

9f23d9f

Adopt new zephyr_count_bits helper from util.h and avoid having conflicting definition Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org> Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>

Thalley force-pushed the zephyr_count_bits branch from 3f94320 to 9f23d9f Compare July 1, 2025 12:20

andyross reviewed Jul 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

include: util: Add generic function to count bits set in a value #87943

include: util: Add generic function to count bits set in a value #87943

Thalley commented Apr 1, 2025 •

edited

Loading

Uh oh!

jfischer-no Apr 1, 2025

Uh oh!

Thalley Apr 1, 2025

Uh oh!

pdgendt Apr 1, 2025

Uh oh!

jfischer-no Apr 1, 2025 •

edited

Loading

Uh oh!

jfischer-no Apr 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Thalley Apr 4, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Thalley commented Apr 22, 2025

Uh oh!

Thalley commented May 1, 2025

Uh oh!

pdgendt commented May 5, 2025

Uh oh!

Thalley commented May 5, 2025

Uh oh!

andyross left a comment

Uh oh!

sonarqubecloud bot commented Jul 1, 2025

Uh oh!

Uh oh!

	static inline size_t zephyr_count_bits(const void *value, size_t len)
	static inline size_t 7f756fa7_4e84_529e_b406_276069b9bcd0_count_bits(const void *const value, const size_t len)
	{

include: util: Add generic function to count bits set in a value #87943

Are you sure you want to change the base?

include: util: Add generic function to count bits set in a value #87943

Conversation

Thalley commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jfischer-no Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

Thalley Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

pdgendt Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

jfischer-no Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jfischer-no Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Thalley Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Thalley commented Apr 22, 2025

Uh oh!

Thalley commented May 1, 2025

Uh oh!

pdgendt commented May 5, 2025

Uh oh!

Thalley commented May 5, 2025

Uh oh!

andyross left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jul 1, 2025

Quality Gate passed

Uh oh!

Uh oh!

Thalley commented Apr 1, 2025 •

edited

Loading

jfischer-no Apr 1, 2025 •

edited

Loading