Re: Trying out <stdatomic.h>

Hello Hackers.

Attached is v5 rebased on 33b392eaabd. I've renamed the 0002 and 0003 patches marked "NOT FOR MERGE" with a trailing "_" to (hopefully) allow the CI to run with only the single patch that adds the optional stdatomic path. The benchmark patch is just to document how I tested/compared the two options and the final patch is for consideration at some point in the future should the 0001 patch be committed. Yes, I've used AI/LLMs to help investigate, test, and write up this set of changes. I've review them myself too and feel they represent something worth reviewing. Thanks in advance for your time, I'll include the LLM-generated in-depth summary of the changes below for reference sake.

Summary:

- It is opt-in and OFF by default: -Duse_stdatomic=no (the default)
keeps the traditional platform-specific atomics. This is an
alternative implementation, not a replacement.
- No measurable regression on x86-64; parity-to-slightly-faster on
aarch64 (numbers below).
- Functionally correct on x86-64, aarch64, RISC-V, FreeBSD, and
Windows-on-ARM (MSVC), on both build systems (meson/autoconf).
- Requires a working C11 <stdatomic.h>: MSVC 2022+ (with
/experimental:c11atomics), GCC 4.9+/Clang 3.1+. Anything older
just builds the traditional path.

The series is three patches:

1. The whole implementation -- build-time detection (meson +
autoconf), the C11 code in port/atomics/stdatomic_impl.h, and the
wire-up (atomics.h / storage/spin.h / s_lock.c). This is the only
patch proposed for commit; it defaults to "no".
2. [NOT FOR MERGE] the benchmark harness, for transparency/repro.
3. [NOT FOR MERGE... yet.] removes the traditional implementation
entirely (arch-*.h, generic-*.h, fallback.h, s_lock.h) and makes
stdatomic the sole path.

Per-operation mapping
---------------------
Every hand-written pg_atomic entry point and how it maps to C11. "trad"
is what the existing arch-*/generic-* code effectively provides; the
mapping column is the atomic_*_explicit order this series emits.

operation | trad semantics | stdatomic map | notes
-------------------------+-------------------+---------------+------
read_u32/u64 | plain, no barrier | seq_cst load | (1)
write_u32/u64 | plain, no barrier | relaxed store | (1)
unlocked_write_u32/u64 | plain, no atomic | relaxed store | (2)
read_membarrier_u32/u64 | read + full fence | seq_cst load |
write_membarrier_u32/u64 | write + full fence| seq_cst store |
init_u32/u64 | plain store | atomic_init | (3)
-------------------------+-------------------+---------------+------
exchange_u32/u64 | full barrier | seq_cst xchg |
compare_exchange_u32/u64 | full barrier | seq_cst CAS | (4)
-------------------------+-------------------+---------------+------
fetch_add/sub/and/or_* | full barrier | seq_cst RMW |
add_fetch/sub_fetch_* | full barrier | RMW + arith | (5)
monotonic_advance_u64 | CAS loop | CAS loop | (6)
-------------------------+-------------------+---------------+------
init_flag | store 0/unlocked | atomic_init | (7)
test_set_flag | TAS, acquire | fetch_and,acq | (7)
unlocked_test_flag | plain read | relaxed load | (7)
clear_flag | store, release | release store | (7)
-------------------------+-------------------+---------------+------
pg_compiler_barrier | compiler barrier | signal_fence | (8)
pg_read_barrier | read/acquire | signal+acquire| (8)
pg_write_barrier | write/release | signal+release| (8)
pg_memory_barrier | full barrier | signal+seq_cst| (8)
pg_spin_delay | pause/isb/yield | unchanged |

(1) The crux of this mail -- see "read/write ordering" below. The
write is relaxed (plain STR, matching the "no barrier" contract);
the read is seq_cst, for a reason worth reviewing.
(2) unlocked_write is a plain non-atomic store when exclusive access
is guaranteed; relaxed matches.
(3) init is not atomic; only legal before publication. (4) strong CAS,
seq_cst on both success and failure paths. (5) C11 has no
add_fetch/sub_fetch; compose from fetch_* + arith. (6) unchanged;
built on compare_exchange in atomics.h, no impl needed. (7) flag is
backed by a 32-bit word, not uint8 -- see "flag width".
Internal polarity is inverted (1=unlocked) but the public
contract (test_set returns true on acquire) is identical. See
also the note on Heikki's pg_atomic_bool patch below.
(8) a bare atomic_thread_fence does not by itself constrain the
compiler's movement of plain (non-atomic) accesses, so each
barrier is atomic_signal_fence (compiler barrier) *plus* a thread
fence -- see "barriers".

What the standard atomics do differently, under the covers
----------------------------------------------------------
Mostly this is renaming our names to the standard ones. The non-obvious
differences that matter:

* Barriers. A bare atomic_thread_fence() does not by itself constrain
the compiler from moving plain, non-atomic accesses across it; our
pg_read/write/memory_barrier() must order those too. So each barrier
is a compiler barrier (atomic_signal_fence) *plus*
atomic_thread_fence(acquire/release/seq_cst). Miss the compiler
barrier and the optimizer will happily reorder non-atomic loads/stores
across it.

* Flag width. Standard atomic_flag has no relaxed load, so like
Thomas's patch I build pg_atomic_flag on an integer. It must be
32-bit, not 8-bit: RISC-V has no byte-granular AMO, so an 8-bit
fetch_and is emulated as a read-modify-write of the whole containing
word, which clobbers adjacent bytes in a packed slock_t. (generic.h
already uses uint32 for exactly this reason.) Note Heikki's in-flight
"Replace pg_atomic_flag with pg_atomic_bool" patch removes
pg_atomic_flag entirely; I'll rebase onto pg_atomic_bool rather than
carry the flag, which also makes the polarity note (7) moot.

* RMW ops (fetch_add, compare_exchange, exchange) are seq_cst in both
the old and new worlds, so those are unchanged.

* Lock-free, asserted. PostgreSQL's atomics live in shared memory and
are touched from multiple processes, not just threads (forshadowing).
A non-lock-free _Atomic that the compiler lowers to a libatomic lock
table would use process-local locks and silently corrupt shared
state. So this asserts at compile time, per width, that the atomics
are genuinely lock-free
(StaticAssertDecl(ATOMIC_{CHAR,SHORT,INT,LLONG}_LOCK_FREE == 2, ...),
skipped only for MSVC-in-C-mode where the macro is conservatively 1),
as Thomas's patch did. Relatedly the configure/meson probe is a
*link* test with a -latomic fallback, not just a compile test: a
64-bit compare-exchange can lower to a libatomic call
(__atomic_compare_exchange_8) on some 32-bit targets, where a
compile-only check would pass and then fail at final link.

read/write ordering: the part worth reviewing
----------------------------------------------
This is the one place the C11 mapping is not a mechanical rename, and I
want to be transparent about how I arrived at it.

pg_atomic_read_u32/write_u32 are documented as plain atomic (non-torn)
accesses with *no barrier semantics*. The obvious C11 mapping is
memory_order_relaxed for both. That is correct for the write. It is
NOT correct for the read, and here is the evidence.

With relaxed reads, a parallel hash join intermittently loses a tuple on
RISC-V (join_hash "extremely_skewed" returns 19999 instead of 20000),
stdatomic build only, ~1 run in 4 on real rv64 hardware. I bisected the
read variant (join_hash, write relaxed throughout):

read variant join_hash
traditional volatile load (stock) 20/20 PASS
C11 relaxed atomic load 2/20 (broken)
C11 relaxed load + compiler barrier 4/20 (broken)
C11 seq_cst atomic load 15/15 PASS

The top two rows are the surprise: the traditional volatile load passes,
but a C11 memory_order_relaxed load of the same variable fails, on the
same hardware. They are not equivalent. A volatile access cannot be
reordered *by the compiler* relative to the surrounding
barriers/spinlock, so consumers that publish a pointer and then
read+dereference it (e.g. ExecParallelHash reading a bucket head, then
the tuple) get the dependent load ordered for free. A C11 relaxed load
loses that compiler-ordering. seq_cst restores it.

The obvious objection is "then relax the read and add the ordering at
the one hash-join consumer." I tried exactly that -- a
pg_read_barrier() in ExecParallelHashFirstTuple() between the bucket
read and the dereference -- and it was NOT sufficient: RISC-V still lost
a tuple at the same ~1-in-4 rate. The ordering the volatile load
provided is not local to that one read; the parallel hash join also
reaches shared state through the Barrier / condition-variable / LWLock
machinery, all of which read via pg_atomic_read_*() and relied on the
same compiler-ordering. Relaxing the primitive loses it everywhere at
once; re-establishing it consumer by consumer would mean auditing many
call sites across lmgr/ipc/executor. So the read stays seq_cst -- the
single change that restores, tree-wide, the property the volatile load
had.

Two clarifications so nobody over-reads this:

- This is NOT a latent bug in master. I built and ran stock master
on the same rv64 box: join_hash passed 40/40, zero lost tuples.
Master's volatile load + hardware address-dependency is sufficient
there. The seq_cst read compensates specifically for what the C11
*relaxed* mapping would otherwise lose; it is not fixing a
pre-existing defect.
- A seq_cst read paired with a relaxed write does not by itself
establish a total order over read and write. The read's seq_cst is
there to restore the compiler-ordering the old volatile read had,
which is what the consumers depended on -- not to claim a read/write
total order.

If someone sees how to return the generic read to relaxed without a
tree-wide consumer audit, I'd genuinely like to know; I could not find
one that survives RISC-V (a target I'll champion even if others don't
yet find it viable).

Why the write is relaxed (and why seq_cst-everywhere was wrong)
---------------------------------------------------------------
My first cut made read AND write seq_cst -- and that showed a real
~1.5-2% aarch64 read-only pgbench regression. On aarch64 a seq_cst
store compiles to STLR; a plain write is STR. Under cross-core
contention on a hot line (LWLock state on release, buffer header state)
STLR drains the store buffer and joins the total store order,
serializing writers. A contended-write microbench (yes = stdatomic,
threads hammering one line):

threads yes ns no ns ratio
2 0.427 0.256 1.67x
4 0.392 0.130 3.02x
8 0.482 0.103 4.68x
16 0.471 0.099 4.76x

Uncontended, single-threaded, read and write are identical between the
two builds (LDAR/STLR in isolation cost nothing here). So the fix is to
keep the WRITE relaxed (plain STR) and pay ordering only on the READ,
whose seq_cst LDAR is free uncontended and correct where it matters.
x86 is immune throughout: on TSO a seq_cst load is a plain mov.

Numbers
-------
Two bare-metal EC2 instances, 192 vCPU / 2 NUMA nodes each: Intel
Sapphire Rapids (m7i.metal-48xl) and Graviton (c9g.metal-48xl). Turbo
off, governor=performance, THP off, numa_balancing off, explicit
hugepages. Both variants built from one checkout, same compiler/flags,
only -Duse_stdatomic differing; buildtype=release, cassert=off.
Cache-resident (scale 100, shared_buffers 32-64GB), fsync=off,
synchronous_commit=off to isolate the lock/atomic paths. pgbench -M
prepared, -S (RO) and -N (RW). A/B alternated; median tps, bootstrap 95%
CI, Mann-Whitney U.

x86-64, RO and RW: parity. Every cell within +/-0.5% of stock, MAD
~0.1%; the few p<0.05 cells are noise-floor (both directions). As
expected on TSO.

aarch64 read-only, seq_cst read + relaxed write (the shipped choice),
backends on node 0, clients on node 1, 8 x 45s runs/point, ratio =
stdatomic/stock, Mann-Whitney p:

clients yes tps no tps ratio p
1 76,931 75,195 1.023 0.009
4 291,624 289,523 1.007 0.021
16 1,142,676 1,128,195 1.013 0.001
48 3,117,124 3,056,956 1.020 0.001
96 3,406,632 3,408,343 1.000 0.529

Parity-to-slightly-faster on aarch64. (The 192-client cell dips below
1.0 on both arches, but that's backends oversubscribed past one node's
core count, not an atomics effect.) aarch64 write (-N) is within noise.
For contrast, the earlier blanket-seq_cst first cut was consistently
~1.5-2% slower on aarch64 RO (p<0.001) -- that regression is what moving
the write back to relaxed erased.

I have the raw CSVs, per-cell stats, and microbench data and can share
them.

Portability / correctness notes that apply to any <stdatomic.h> port
--------------------------------------------------------------------
- Pair every thread fence with a compiler barrier; a bare
atomic_thread_fence does not order plain accesses against the
compiler.
- Make pg_atomic_flag a 32-bit word (RISC-V byte-AMO emulation).
- A C11 memory_order_relaxed load is not the same as a volatile load:
it loses compiler-ordering. If a consumer relied on the volatile
load's ordering, relaxed will bite on weak-memory hardware.
- Assert lock-freeness per width (cross-process shared memory), and
use a link test with -latomic fallback for the wide CAS.

best.

-greg