Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix CPU-identification macros for RISC-V.

  2. Clean up inconsistencies in CPU-identification macros.

  1. Centralised architecture detection

    Thomas Munro <thomas.munro@gmail.com> — 2026-04-09T01:01:41Z

    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

  2. Re: Centralised architecture detection

    Thomas Munro <thomas.munro@gmail.com> — 2026-04-09T02:34:01Z

    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.

  3. Re: Centralised architecture detection

    John Naylor <johncnaylorls@gmail.com> — 2026-04-09T08:16:33Z

    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

  4. Re: Centralised architecture detection

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2026-04-09T10:29:22Z

    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

  5. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-03T19:24:22Z

    =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:
> 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}

Nathan Bossart reminded me of this thread after I'd independently
rediscovered the same thing [1].  I agree with standardizing on
just one spelling of these CPU-type macros.  But I wonder why we
should invent our own instead of standardizing on gcc's spellings
(that is, __x86_64__ etc).  The amount of code churn required for
this patch would drop drastically if we did it that way.  And I
suspect it would be less likely that we'd need to fixup future patch
submissions than if we have a homegrown standard.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/3035145.1780503430%40sss.pgh.pa.us

  6. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-03T21:08:55Z

    I wrote:
> Nathan Bossart reminded me of this thread after I'd independently
> rediscovered the same thing [1].  I agree with standardizing on
> just one spelling of these CPU-type macros.  But I wonder why we
> should invent our own instead of standardizing on gcc's spellings
> (that is, __x86_64__ etc).  The amount of code churn required for
> this patch would drop drastically if we did it that way.  And I
> suspect it would be less likely that we'd need to fixup future patch
> submissions than if we have a homegrown standard.

Concretely, I'm imagining that we'd do more or less the attached in
c.h, and then the rest of the work would just be to remove the
not-very-large number of references to the alternative CPU symbols.

Note that I threw in an "#else #error" branch to ensure that we
successfully identify every architecture.  Even if you don't like
this naming scheme, we should do that with the original patch too.

			regards, tom lane

  7. Re: Centralised architecture detection

    Nico Williams <nico@cryptonector.com> — 2026-06-03T21:21:14Z

    On Wed, Jun 03, 2026 at 05:08:55PM -0400, Tom Lane wrote:
> I wrote:
> > Nathan Bossart reminded me of this thread after I'd independently
> > rediscovered the same thing [1].  I agree with standardizing on
> > just one spelling of these CPU-type macros.  But I wonder why we
> > should invent our own instead of standardizing on gcc's spellings
> > (that is, __x86_64__ etc).  The amount of code churn required for
> > this patch would drop drastically if we did it that way.  And I
> > suspect it would be less likely that we'd need to fixup future patch
> > submissions than if we have a homegrown standard.
> 
> Concretely, I'm imagining that we'd do more or less the attached in
> c.h, and then the rest of the work would just be to remove the
> not-very-large number of references to the alternative CPU symbols.

Can a pre-processor make it an error for users to define __ macros?

  8. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-03T21:27:22Z

    Nico Williams <nico@cryptonector.com> writes:
> On Wed, Jun 03, 2026 at 05:08:55PM -0400, Tom Lane wrote:
>> Concretely, I'm imagining that we'd do more or less the attached in
>> c.h, and then the rest of the work would just be to remove the
>> not-very-large number of references to the alternative CPU symbols.

> Can a pre-processor make it an error for users to define __ macros?

I don't believe so.  We have done similar things elsewhere, eg
before 25f36066d we had this in solaris.h:

/*
 * Sort this out for all operating systems some time.  The __xxx
 * symbols are defined on both GCC and Solaris CC, although GCC
 * doesn't document them.  The __xxx__ symbols are only on GCC.
 */
#if defined(__i386) && !defined(__i386__)
#define __i386__
#endif

#if defined(__amd64) && !defined(__amd64__)
#define __amd64__
#endif

#if defined(__x86_64) && !defined(__x86_64__)
#define __x86_64__
#endif

#if defined(__sparc) && !defined(__sparc__)
#define __sparc__
#endif

Of course that only proves that Sun Studio didn't complain.

			regards, tom lane

  9. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-16T17:57:34Z

    I wrote:
> Concretely, I'm imagining that we'd do more or less the attached in
> c.h, and then the rest of the work would just be to remove the
> not-very-large number of references to the alternative CPU symbols.

Here's a fleshed-out (and now actually lightly-tested) version
of that.

			regards, tom lane

  10. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-30T16:22:12Z

    > I wrote:
>> Concretely, I'm imagining that we'd do more or less the attached in
>> c.h, and then the rest of the work would just be to remove the
>> not-very-large number of references to the alternative CPU symbols.

> Here's a fleshed-out (and now actually lightly-tested) version
> of that.

Pushed.  I shall now watch the buildfarm from a safe distance.

			regards, tom lane

  11. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-30T21:31:47Z

    I wrote:
> Pushed.  I shall now watch the buildfarm from a safe distance.

Sure enough, greenfly is not happy:

ccache /scratch/opt/llvm-22/bin/clang -Isrc/port/libpgport_srv.a.p -Isrc/include -I../pgsql/src/include -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O2 -g -fno-strict-aliasing -fwrapv -fexcess-precision=standard -D_GNU_SOURCE -Wpointer-arith -Werror=vla -Werror=unguarded-availability-new -Wmissing-format-attribute -Wcast-function-type -Wformat-security -Wmissing-prototypes -Wold-style-definition -Wstrict-prototypes -Wimplicit-fallthrough -Wdeclaration-after-statement -Wmissing-variable-declarations -Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -Wno-format-truncation -Wno-cast-function-type-strict -march=rv64gcv -fPIC -DBUILDING_DLL -MD -MQ src/port/libpgport_srv.a.p/bsearch_arg.c.o -MF src/port/libpgport_srv.a.p/bsearch_arg.c.o.d -o src/port/libpgport_srv.a.p/bsearch_arg.c.o -c ../pgsql/src/port/bsearch_arg.c
In file included from ../pgsql/src/port/bsearch_arg.c:36:
../pgsql/src/include/c.h:162:2: error: "cannot identify target architecture"
  162 | #error "cannot identify target architecture"
      |  ^
1 error generated.

So I was wrong to guess that every riscv64 platform predefines
__riscv64__.  Greg, could you check what predefined architecture
symbols that compiler does supply?  I'm tempted to blindly guess
that __riscv64 will work, but I'd rather not guess.

To save you having to look it up, something like this should
do the trick:

clang -dM -E - </dev/null | sort >clang-predefined-macros

			regards, tom lane

  12. Re: Centralised architecture detection

    Thomas Munro <thomas.munro@gmail.com> — 2026-07-01T00:31:40Z

    On Wed, Jul 1, 2026 at 9:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> So I was wrong to guess that every riscv64 platform predefines
> __riscv64__.  Greg, could you check what predefined architecture
> symbols that compiler does supply?  I'm tempted to blindly guess
> that __riscv64 will work, but I'd rather not guess.

No riscv here, but poking around, it looks like it's supposed to be
__riscv, and then __riscv_xlen == 32 or 64, or perhaps pointer size
check?  Not like the others, but this seems to have come down from the
RISCV project.

https://lists.riscv.org/g/sig-toolchains/attachment/688/0/riscv-toolchain.pdf
https://github.com/riscv-non-isa/riscv-c-api-doc/blob/main/src/c-api.adoc

It looks like they didn't want __riscv32 and __riscv64?

https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/E8EO-Fd4t3s

  13. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-01T01:11:38Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> It looks like they didn't want __riscv32 and __riscv64?
> https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/E8EO-Fd4t3s

Sigh ... another project that is convinced that they're smarter than
everybody else and conforming to common practice is an anti-pattern.

Based on that thread, I'm thinking

...
#elif defined(__riscv)
#if SIZEOF_VOID_P == 8
#define __riscv64__ 1
#else
#define __riscv__ 1
#endif
#elif defined(__s390__)
...

I'd rather rely on our own pointer-size determination than YA
magic compiler-defined symbol.

			regards, tom lane

  14. Re: Centralised architecture detection

    Greg Burd <greg@burd.me> — 2026-07-01T11:39:40Z

    On Tue, Jun 30, 2026, at 5:31 PM, Tom Lane wrote:
> I wrote:
>> Pushed.  I shall now watch the buildfarm from a safe distance.
>
> Sure enough, greenfly is not happy:

That's a good thing, and the reason it exists. :)

> ccache /scratch/opt/llvm-22/bin/clang -Isrc/port/libpgport_srv.a.p 
> -Isrc/include -I../pgsql/src/include -fdiagnostics-color=always 
> -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O2 -g -fno-strict-aliasing 
> -fwrapv -fexcess-precision=standard -D_GNU_SOURCE -Wpointer-arith 
> -Werror=vla -Werror=unguarded-availability-new 
> -Wmissing-format-attribute -Wcast-function-type -Wformat-security 
> -Wmissing-prototypes -Wold-style-definition -Wstrict-prototypes 
> -Wimplicit-fallthrough -Wdeclaration-after-statement 
> -Wmissing-variable-declarations -Wno-unused-command-line-argument 
> -Wno-compound-token-split-by-macro -Wno-format-truncation 
> -Wno-cast-function-type-strict -march=rv64gcv -fPIC -DBUILDING_DLL -MD 
> -MQ src/port/libpgport_srv.a.p/bsearch_arg.c.o -MF 
> src/port/libpgport_srv.a.p/bsearch_arg.c.o.d -o 
> src/port/libpgport_srv.a.p/bsearch_arg.c.o -c 
> ../pgsql/src/port/bsearch_arg.c
> In file included from ../pgsql/src/port/bsearch_arg.c:36:
> ../pgsql/src/include/c.h:162:2: error: "cannot identify target 
> architecture"
>   162 | #error "cannot identify target architecture"
>       |  ^
> 1 error generated.
>
> So I was wrong to guess that every riscv64 platform predefines
> __riscv64__.  Greg, could you check what predefined architecture
> symbols that compiler does supply?  I'm tempted to blindly guess
> that __riscv64 will work, but I'd rather not guess.
>
> To save you having to look it up, something like this should
> do the trick:
>
> clang -dM -E - </dev/null | sort >clang-predefined-macros

Attached, happy to help out.

> 			regards, tom lane

best.

-greg

  15. Re: Centralised architecture detection

    Greg Burd <greg@burd.me> — 2026-07-01T11:43:16Z

    On Tue, Jun 30, 2026, at 9:11 PM, Tom Lane wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
>> It looks like they didn't want __riscv32 and __riscv64?
>> https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/E8EO-Fd4t3s
>
> Sigh ... another project that is convinced that they're smarter than
> everybody else and conforming to common practice is an anti-pattern.
>
> Based on that thread, I'm thinking
>
> ...
> #elif defined(__riscv)
> #if SIZEOF_VOID_P == 8
> #define __riscv64__ 1
> #else
> #define __riscv__ 1
> #endif
> #elif defined(__s390__)
> ...

I don't see SIZEOF_VOID_P but I do find:

#define __SIZEOF_POINTER__ 8

-greg

> I'd rather rely on our own pointer-size determination than YA
> magic compiler-defined symbol.
>
> 			regards, tom lane

  16. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-01T13:20:32Z

    "Greg Burd" <greg@burd.me> writes:
> On Tue, Jun 30, 2026, at 9:11 PM, Tom Lane wrote:
>> Based on that thread, I'm thinking
>> #if SIZEOF_VOID_P == 8

> I don't see SIZEOF_VOID_P but I do find:
> #define __SIZEOF_POINTER__ 8

SIZEOF_VOID_P is set up by autoconf/meson via pg_config.h.
So it will be available here.  I'd rather use our own
symbol because, if other arches emerge with similar issues,
we can be sure of having a common pattern to follow.

I do see __SIZEOF_POINTER__ getting predefined locally
(in recent gcc and clang on x86_64), but it's far from clear
to me how standard, or well-documented, that macro is.

			regards, tom lane

  17. Re: Centralised architecture detection

    Greg Burd <greg@burd.me> — 2026-07-03T15:28:50Z

    On Wed, Jul 1, 2026, at 9:20 AM, Tom Lane wrote:
> "Greg Burd" <greg@burd.me> writes:
>> On Tue, Jun 30, 2026, at 9:11 PM, Tom Lane wrote:
>>> Based on that thread, I'm thinking
>>> #if SIZEOF_VOID_P == 8
>
>> I don't see SIZEOF_VOID_P but I do find:
>> #define __SIZEOF_POINTER__ 8
>
> SIZEOF_VOID_P is set up by autoconf/meson via pg_config.h.

facepalm, of course! :)

> So it will be available here.  I'd rather use our own
> symbol because, if other arches emerge with similar issues,
> we can be sure of having a common pattern to follow.
>
> I do see __SIZEOF_POINTER__ getting predefined locally
> (in recent gcc and clang on x86_64), but it's far from clear
> to me how standard, or well-documented, that macro is.
>
> 			regards, tom lane

Was there anything else you needed from me on this one?

best.

-greg

  18. Re: Centralised architecture detection

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-07-03T15:36:42Z

    "Greg Burd" <greg@burd.me> writes:
> Was there anything else you needed from me on this one?

Nope, looks like all the riscv BF animals are green again,
so we're good.  Thanks for helping!

			regards, tom lane