Re: refactor architecture-specific popcount code
John Naylor <johncnaylorls@gmail.com>
From: John Naylor <johncnaylorls@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: pgsql-hackers@postgresql.org
Date: 2026-01-15T09:07:51Z
Lists: pgsql-hackers
On Thu, Jan 15, 2026 at 3:40 AM Nathan Bossart <nathandbossart@gmail.com> wrote: > Right now, the organization of this code is weird. All AArch64-specific > implementations live in an AArch64-specific file, the AVX-512 > implementations live in their own file, and the architecture-agnostic and > SSE4.2 implementations live in pg_bitutils.c. The attached patches move > the SSE4.2 implementations to the AVX-512 file (which is renamed > appropriately), and they update some function names to be more descriptive, > i.e., "fast" is replaced with "sse42" and "slow" is replaced with > "generic". Thanks for taking on some technical debt! 0001 --- a/src/port/pg_popcount_avx512.c +++ b/src/port/pg_popcount_x86_64.c Can we get away with just "x86" for brevity? We generally don't target 32-bit CPUs for this kind of work, so there's no chance of confusion. 0002 ``` -#ifdef USE_AVX512_POPCNT_WITH_RUNTIME_CHECK +#include "port/pg_bitutils.h" + +#ifdef TRY_POPCNT_X86_64 #if defined(HAVE__GET_CPUID) || defined(HAVE__GET_CPUID_COUNT) #include <cpuid.h> #endif ``` With the above in the x86 .c file, I wonder we can get rid of this stanza and the "try" symbol and gate only on HAVE_X86_64_POPCNTQ: #ifdef HAVE_X86_64_POPCNTQ #if defined(HAVE__GET_CPUID) || defined(HAVE__CPUID) #define TRY_POPCNT_X86_64 1 #endif #endif If we have to be cautious, we could just turn the #error on no CPUID symbol into "return false". 0003 s/fast/sse42/: Seems okay in this file, but this isn't the best name, either. Maybe a comment to head off future "corrections", something like: "Technically, POPCNT is not part of SSE 4.2, and is not even a vector operation, but many compilers emit the popcnt instruction with -msse4.2 anyway." s/slow/generic/: I'm ambivalent about this. The "slow" designation is flat-out wrong since at least Power and aarch64 can emit a single instruction here without prodding the compiler. On the other hand, "generic" seems wrong too, since e.g. pg_popcount64_slow() has three configure symbols and two compiler builtins. :-D A possible future project would be to have a truly generic simple fallback in pure C and put all the fancy stuff in the header for architectures that have unconditional hardware support. It would make more sense to revisit the name then. -- John Naylor Amazon Web Services
Commits
-
Make use of pg_popcount() in more places.
- f33b8793fd73 19 (unreleased) landed
-
Remove uses of popcount builtins.
- eb9ab7e0930a 19 (unreleased) landed
-
Remove some unnecessary optimizations in popcount code.
- cb7b2e5e8efb 19 (unreleased) landed
-
Remove specialized word-length popcount implementations.
- a4688988835f 19 (unreleased) landed
-
Move x86-64-specific popcount code to pg_popcount_x86.c.
- 79e232ca013c 19 (unreleased) landed
-
Refactor some SIMD and popcount macros.
- 25dc4850747b 19 (unreleased) landed
-
Rename "fast" and "slow" popcount functions.
- 8c6653516c5a 19 (unreleased) landed
-
Rename pg_popcount_avx512.c to pg_popcount_x86.c.
- fbe327e5b465 19 (unreleased) landed
-
Remove trailing zero words from Bitmapsets
- a8c09daa8bb1 17.0 cited
-
Don't use _BitScanForward64/_BitScanReverse64 on 32-bit MSVC builds
- 53ea2b7ad050 16.0 cited