From: Nathan Bossart <nathandbossart@gmail.com>
To: John Naylor <johncnaylorls@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: refactor architecture-specific popcount code
Date: Thu, 15 Jan 2026 09:59:17 -0600
Message-ID: <aWkO1TG5gWLhUtAJ@nathan> (raw)
In-Reply-To: <CANWCAZb0U2qDTHCxPDyZugOTSE7xPURWKwcf8d4JmYTgH4+nEA@mail.gmail.com>
References: <aWf_InS1VrbeXAfP@nathan>
<CANWCAZb0U2qDTHCxPDyZugOTSE7xPURWKwcf8d4JmYTgH4+nEA@mail.gmail.com>
On Thu, Jan 15, 2026 at 04:07:51PM +0700, John Naylor wrote:
> Thanks for taking on some technical debt!
Thanks for reviewing.
> --- 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.
WFM.
> ```
> -#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".
Yeah, the CPUID macro checks do seem overly cautious to me, especially
since we've just #error'd when the CPUID intrinsics are missing in
pg_crc32c_sse42_choose.c since 2015. That seems to suggest that nobody is
trying to build Postgres with a compiler that knows about SSE4.2/POPCNT but
not CPUID. For reference, CPUID was introduced in 1993.
I bet we could also convert this bit into a configuration-time check:
#if defined(_MSC_VER) && defined(_M_AMD64)
#define HAVE_X86_64_POPCNTQ
#endif
> 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."
Makes sense.
--
nathan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, johncnaylorls@gmail.com
Subject: Re: refactor architecture-specific popcount code
In-Reply-To: <aWkO1TG5gWLhUtAJ@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox