Обсуждение: Centralised architecture detection

Hi,

Catching up with the timing_clock_source thread (great work!), I saw
that Andres mentioned that it'd be nice to have PG_ARCH_X86 etc macros
to standardise our detection of CPUs[1], and I remembered that I'd
already looked into that a couple of years ago and posted a patch...
and promptly forgot all about it.  Then I remembered the surprising
discovery that motivated it[2], picked up in passing while working on
early versions of my <stdatomic.h> patch, that we still haven't done
anything about:

We are not detecting x86 in several places on Visual Studio, and the
one where we fail to include src/port/atomics/arch-x86.h can't be too
great for performance.

     * pg_{read,write}_barrier() → pg_memory_barrier()!
     * pg_spin_delay() → nothing!
     * PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY → undefined

No Windows here but it's easy enough to confirm with CI: add #error to
arch-x86.h and you'll get a red MinGW task and a green Visual Studio
task.  Presumably the Windows support was either written blind or
developed on MinGW.

Here's an update of my old patch.  It just defines macros like this,
in c.h, though since then we gained port/pg_cpu.h, so perhaps it
belongs in there.  Then we can deal with the underlying
compiler-defined macros in one central place.  The names I came up
with were:

      PG_ARCH_{ARM,LOONGARCH,MIPS,PPC,RISCV,S390,SPARC,X86}
      PG_ARCH_{ARM,LOONGARCH,MIPS,PPC,RISCV,S390,SPARC,X86}_{32,64}

Lukas and John have both been doing similar sorts of things and may
have better ideas or patches, but I figured I should at least re-post
what I have.

I suppose we could also do something similar for PG_COMPILER_XXX and
PG_OS_XXX.  I've made mistakes with those before too...

[1] https://www.postgresql.org/message-id/flat/r5snevsnkyoifjqldu6gcssbnrnezpplq4ofcmekjfvzzu32dc%25405rn26itd4ubr
[2]
https://www.postgresql.org/message-id/flat/CA%2BhUKGKAf_i6w7hB_3pqZXQeqn%2BixvY%2BCMps_n%3DmJ5HAatMjMw%40mail.gmail.com#4ec27ce4c67390c29528f5ef064c8b68

Here's a better version (I'd omitted _M_AMD64 in the previous one
after testing something...).  CC'ing Bryan who might be interested in
the effects of this stuff on Windows builds.

On Thu, Apr 9, 2026 at 8:02 AM Thomas Munro <thomas.munro@gmail.com> wrote:
> Here's an update of my old patch.  It just defines macros like this,
> in c.h, though since then we gained port/pg_cpu.h, so perhaps it
> belongs in there.

port/pg_cpu.h would seem like the natural place for something like
this, and I deliberately made that header's name not specific to one
architecture. But see below.

> Lukas and John have both been doing similar sorts of things and may
> have better ideas or patches, but I figured I should at least re-post
> what I have.

From that thread, I think the final committed version ended up with
fewer places that cared about architecture macros, and that's why it
was left out. I for one was not motivated to continue that work, but I
don't see a reason not to, either. And as you said, this might help
avoid errors of omission going forward.

--- a/src/port/pg_crc32c_sse42.c
+++ b/src/port/pg_crc32c_sse42.c
@@ -39,7 +39,7 @@ pg_comp_crc32c_sse42(pg_crc32c crc, const void
*data, size_t len)
  * and performance testing didn't show any performance gain from aligning
  * the begin address.
  */
-#ifdef __x86_64__
+#ifdef PG_ARCH_X86_64
  while (p + 8 <= pend)

That probably should have been "SIZEOF_VOID_P >= 8" to begin with.

Also, src/port/pg_cpu_x86.c currently has this hack:

#if defined(USE_SSE2) || defined(__i386__)

That's an awkward way of saying "x86 of any word size, but forget
about 32-bit MSVC because it won't get tested in the buildfarm", and
should probably use PG_ARCH_X86 from this patch. However, as currently
written, that would only work if the new macros were in c.h, to keep
the property that system headers come before (most) PG headers.

--
John Naylor
Amazon Web Services

Thomas Munro <thomas.munro@gmail.com> writes:

> Instead of repeating compilers' architecture macros throughout the tree
> and sometimes getting it wrong, let's detect them in one central place,
> and define our own macros of the form:
>
>   PG_ARCH_{ARM,LOONGARCH,MIPS,PPC,RISCV,S390,SPARC,X86}
>   PG_ARCH_{ARM,LOONGARCH,MIPS,PPC,RISCV,S390,SPARC,X86}_{32,64}
[...]
> diff --git a/contrib/pgcrypto/crypt-blowfish.c b/contrib/pgcrypto/crypt-blowfish.c
> index 5a1b1e10091..9c4e02e428b 100644
> --- a/contrib/pgcrypto/crypt-blowfish.c
> +++ b/contrib/pgcrypto/crypt-blowfish.c
> @@ -38,10 +38,10 @@
>  #include "px-crypt.h"
>  #include "px.h"
>  
> -#ifdef __i386__
> +#if defined(PG_ARCH_X86_32)
>  #define BF_ASM                0    /* 1 */
>  #define BF_SCALE            1
> -#elif defined(__x86_64__)
> +#elif defined(PG_ARCH_X86_64)
>  #define BF_ASM                0
>  #define BF_SCALE            1
>  #else

These could be combined into a single #ifdef PG_ARCH_X86.  Also, BF_ASM
has never been defined to anything but 0 since this file was added in
2001, and the _BF_body_r function it would call in that case has never
existed, so we could get rid of it (but that would be for a separate
patch).

- ilmari