On 2024-Jul-01, Tom Lane wrote: > Alvaro Herrera <alvhe...@alvh.no-ip.org> writes: > > Maybe we can do something like this, > > > +#if MAXIMUM_ALIGNOF >= 8 > > uint64 currval; > > This should probably be testing the alignment of int64 specifically, > rather than assuming that MAXIMUM_ALIGNOF applies to it. At least > historically, there have been platforms where MAXIMUM_ALIGNOF is > determined by float quantities and integer quantities are different.
OK, so it's #if ALIGNOF_LONG_LONG_INT >= 8 Alexander, can you please confirm whether this works for you? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "La virtud es el justo medio entre dos defectos" (Aristóteles)
>From 65e9e813859fec436805b1ada7be9339f8ecd63e Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Mon, 1 Jul 2024 16:44:22 +0200 Subject: [PATCH v4] Fix alignment in pg_atomic_monotonic_advance_u64 Unsurprisingly, it's possible for Windows x86 to have 8-byte atomics but 4-byte-aligned long long ints. This breaks an assertion in recently introduced pg_atomic_monotonic_advance_u64 with commit f3ff7bf83bce. Try to fix that blindly by adding pg_attribute_aligned(8). Unfortunately not all compilers support that, so it needs to be protected by #ifdef; fortunately not all compilers _need_ it. Hopefully the combinations we now support cover all interesting cases. Reported-by: Alexander Lakhin <exclus...@gmail.com> Reviewed-by: Tom Lane <t...@sss.pgh.pa.us> Discussion: https://postgr.es/m/36796438-a718-cf9b-2071-b2c1b947c...@gmail.com --- src/include/port/atomics.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h index 78987f3154..eeb47dee30 100644 --- a/src/include/port/atomics.h +++ b/src/include/port/atomics.h @@ -580,7 +580,21 @@ pg_atomic_sub_fetch_u64(volatile pg_atomic_uint64 *ptr, int64 sub_) static inline uint64 pg_atomic_monotonic_advance_u64(volatile pg_atomic_uint64 *ptr, uint64 target_) { + /* + * When using actual (not simulated) atomics, the target variable for + * pg_atomic_compare_exchange_u64 must have suitable alignment. This + * occurs naturally on platforms where long long int is 8-byte aligned. On + * others, we must explicitly coerce the compiler into applying suitable + * alignment, or abort the compile if we cannot. When using simulated + * atomics, the alignment doesn't matter. + */ +#if ALIGNOF_LONG_LONG_INT >= 8 || defined(PG_HAVE_ATOMIC_U64_SIMULATION) uint64 currval; +#elif defined(pg_attribute_aligned) + pg_attribute_aligned(8) uint64 currval; +#else +#error "Must have pg_attribute_aligned or simulated atomics on this platform" +#endif #ifndef PG_HAVE_ATOMIC_U64_SIMULATION AssertPointerAlignment(ptr, 8); -- 2.39.2
