Differential D50995
linuxkpi: Use `__builtin_popcountg()` instead of `bitcount*()` Authored by dumbbell on Jun 23 2025, 9:37 PM. Tags None Referenced Files
Details
The DRM drivers generic code started to use HWEIGHT64() in the definition of an array field in a structure. Therefore, the array size needs to be known at compile time. This was not the case with the HWEIGHT*() macros based on bitcount*(). The use of __builtin_popcountg() solves that problem. Sponsored by: The FreeBSD Foundation
Diff Detail
Event TimelineComment Actions The only one used today is HWEIGHT32 in an assertion: BUILD_BUG_ON(IWL_MAX_UMAC_SCANS > HWEIGHT32(IWL_MVM_SCAN_MASK) ||
IWL_MAX_LMAC_SCANS > HWEIGHT32(IWL_MVM_SCAN_MASK));Comment Actions I believe so too. But I also do not understand to compile-time issue of this change. Our bitcount() is defined as: sys/sys/libkern.h:#define bitcount(x) __bitcount((u_int)(x)) sys/sys/bitcount.h:#define __bitcount(x) __builtin_popcount((unsigned int)(x)) if POPCNT is defined ... and it seems clang only does for X86? Is this a relict of 589b2c1c1e31b48d9f8defe061983500766aa28f preventing us from being more efficient these days? Lastly contrib/llvm-project/libcxx/include/__bit/popcount.h says: "__builtin_popcountg is available since Clang 19 and GCC 14" No idea what our bare minimum is.
Comment Actions Thank you @emaste for the bugfix. I was not sure myself if the + 1 was intended. About the compile-time problem, the DRM drivers from Linux 6.9 failed to compile: the compiler complained that the size of the array had to be known at compile-time. I work on amd64. Perhaps the DRM drivers’ Makefile lack a define or an include somewhere? Because apparently a version not based on runtime code and not on POPCNT was selected. Comment Actions We could also have a guaranteed constant bitcount macro in sys/bitcount.h that HWEIGHT* could use, even when __POPCNT__ is not defined, something like: #if defined(__POPCNT__) # define const_bitcount8(x) bitcount8(x) /* Does this path even matter? */ #else # define const_bitcount8(x) (!!((x) & (1 << 0)) + !!((x) & (1 << 1)) + !!((x) & (1 << 2)) + etc etc) #endif As I understand, HWEIGHT* are always supposed to be run on compile-time constants, so they could just be defined to this const_bitcount macro. I can create a patch for this if this seems like a good solution to you all. Comment Actions Can you rebase this given @emaste 's update. Then I think it should just go in.
Comment Actions
Sorry, I missed your comment… I have no objection for another approach. I will commit this one for now and we can improve it iteratively. | ||||||||||||||||||||||||||||||||||||||||||||