On Sun, Nov 9, 2025 at 8:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ... BTW, I wonder why you did not add pg_compiler_barrier_impl()
> to our other use of __atomic_thread_fence:
>
> #if !defined(pg_memory_barrier_impl)
> # if defined(HAVE_GCC__ATOMIC_INT32_CAS)
> # define pg_memory_barrier_impl() __atomic_thread_fence(__ATOMIC_SEQ_CST)
> # elif defined(__GNUC__)
> # define pg_memory_barrier_impl() __sync_synchronize()
> # endif
> #endif /* !defined(pg_memory_barrier_impl) */
>
> If the problem is that Clang doesn't treat __atomic_thread_fence
> as a compiler barrier, why is this usage safer than the other two?
__sync_synchronize() actually has the same problem, more surprisingly
if you ask me, but I suppose it doesn't seem very likely that they
ever shipped a compiler with the C11-reinterpretation retrofitted
underneath the traditional GCC __sync workalikes that didn't qualify
for the top branch instead. You're quite right about that one though,
and I'm annoyed at myself for missing it. I suspect it might be less
prone to actual problems due to details of the contexts it's used in
(ie luck), a bit like pg_write_barrier() as far I know so far. I'm
looking into the codegen across our branches in practice, but I
imagine that in any case it should be OK as a follow-up fix for the
next release, it's just an additional problem and hopefully only a
hypothetical one... I'm looking into the codegen in practice across
branches, compilers and architectures to see if I can confirm that...
