Hello,
On 2024-Jun-29, Alexander Lakhin wrote:
> It doesn't, but the following works for me:
> static inline uint64
> pg_atomic_monotonic_advance_u64(volatile pg_atomic_uint64 *ptr, uint64
> target_)
> {
> - uint64 currval;
> + pg_attribute_aligned(8) uint64 currval;
>
> because the failed assertion is:
> #ifndef PG_HAVE_ATOMIC_U64_SIMULATION
> AssertPointerAlignment(&currval, 8);
> #endif
Hah, thank you.
In the meantime I noticed that pg_attribute_aligned() is not supported
in every platform/compiler, so for safety sake I think it's better to go
with what we do for PGAlignedBlock: use a union with a double member.
That should be 8-byte aligned on x86 as well, unless I misunderstand.
BTW if things works after this fix, I suggest you get a buildfarm member
running with this configuration. Otherwise it's quite likely that we'll
break it again. Or we could just decide we don't care about this
particular platform ... but AFAICS the buildfarm does have other 32-bit
animals running.
[ looks at buildfarm ]
Oh! while looking at Adder's config, I noticed this line:
Checking for alignment of "double" : 4
So, I do misunderstand doubles.
So my patch is not going to work. There seems to be nothing that has
alignment 8 there, so I guess we're back to using
pg_attribute_aligned() and abort the compile if that doesn't exist.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"La gente vulgar sólo piensa en pasar el tiempo;
el que tiene talento, en aprovecharlo"
>From 9d240e90f87bf8b53bd5d92b623e33419db64717 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Mon, 1 Jul 2024 10:41:06 +0200
Subject: [PATCH v2] Fix alignment of variable in
pg_atomic_monotonic_advance_u64
Reported-by: Alexander Lakhin <exclus...@gmail.com>
Discussion: https://postgr.es/m/36796438-a718-cf9b-2071-b2c1b947c...@gmail.com
---
src/include/port/atomics.h | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h
index 78987f3154..964732e660 100644
--- a/src/include/port/atomics.h
+++ b/src/include/port/atomics.h
@@ -580,30 +580,38 @@ 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_)
{
- uint64 currval;
+ /*
+ * On 32-bit machines, declaring a bare uint64 variable doesn't promise
+ * the alignment we need, so coerce the compiler this way.
+ */
+ union
+ {
+ uint64 u64;
+ double force_align_d;
+ } currval;
#ifndef PG_HAVE_ATOMIC_U64_SIMULATION
AssertPointerAlignment(ptr, 8);
#endif
- currval = pg_atomic_read_u64_impl(ptr);
- if (currval >= target_)
+ currval.u64 = pg_atomic_read_u64_impl(ptr);
+ if (currval.u64 >= target_)
{
pg_memory_barrier();
- return currval;
+ return currval.u64;
}
#ifndef PG_HAVE_ATOMIC_U64_SIMULATION
- AssertPointerAlignment(&currval, 8);
+ AssertPointerAlignment(&currval.u64, 8);
#endif
- while (currval < target_)
+ while (currval.u64 < target_)
{
- if (pg_atomic_compare_exchange_u64_impl(ptr, &currval, target_))
+ if (pg_atomic_compare_exchange_u64_impl(ptr, &currval.u64, target_))
break;
}
- return Max(target_, currval);
+ return Max(target_, currval.u64);
}
#undef INSIDE_ATOMICS_H
--
2.39.2
