On Sun, Nov 9, 2025 at 8:26 AM Tom Lane <[email protected]> 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...
