On Mon, Dec 04, 2023 at 12:18:05PM -0600, Nathan Bossart wrote: > Barring objections or additional feedback, I think I'm inclined to press > forward with this one and commit it in the next week or two. I'm currently > planning to keep the inline assembly, but I'm considering removing the > configuration checks for __atomic_exchange_n() if the availability of > __atomic_compare_exchange_n() seems like a reliable indicator of its > presence. Thoughts?
Concretely, like this. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
>From c5f5502dc9fed267011d0f3cd34522d8b7ed4ed2 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Mon, 4 Dec 2023 15:05:18 -0600 Subject: [PATCH v2 1/1] Optimize pg_atomic_exchange_u[32|64]. Reviewed-by: Andres Freund Discussion: https://postgr.es/m/20231129212905.GA1258737%40nathanxps13 --- src/include/port/atomics/arch-x86.h | 28 ++++++++++++++++++++ src/include/port/atomics/generic-gcc.h | 32 +++++++++++++++++++++++ src/include/port/atomics/generic-msvc.h | 18 +++++++++++++ src/include/port/atomics/generic-sunpro.h | 14 ++++++++++ 4 files changed, 92 insertions(+) diff --git a/src/include/port/atomics/arch-x86.h b/src/include/port/atomics/arch-x86.h index bb84b9bad8..290f3e0c58 100644 --- a/src/include/port/atomics/arch-x86.h +++ b/src/include/port/atomics/arch-x86.h @@ -184,6 +184,20 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, return (bool) ret; } +#define PG_HAVE_ATOMIC_EXCHANGE_U32 +static inline uint32 +pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 newval) +{ + uint32 res; + __asm__ __volatile__( + " lock \n" + " xchgl %0,%1 \n" +: "=q" (res), "=m" (ptr->value) +: "0"(newval), "m" (ptr->value) +: "memory"); + return res; +} + #define PG_HAVE_ATOMIC_FETCH_ADD_U32 static inline uint32 pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_) @@ -221,6 +235,20 @@ pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, return (bool) ret; } +#define PG_HAVE_ATOMIC_EXCHANGE_U64 +static inline uint64 +pg_atomic_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 newval) +{ + uint64 res; + __asm__ __volatile__( + " lock \n" + " xchgq %0,%1 \n" +: "=q" (res), "=m" (ptr->value) +: "0"(newval), "m" (ptr->value) +: "memory"); + return res; +} + #define PG_HAVE_ATOMIC_FETCH_ADD_U64 static inline uint64 pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_) diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h index da04e9f0dc..1a45394923 100644 --- a/src/include/port/atomics/generic-gcc.h +++ b/src/include/port/atomics/generic-gcc.h @@ -176,6 +176,22 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, } #endif +/* + * __sync_lock_test_and_set() only supports setting the value to 1 on some + * platforms, so we only provide an __atomic implementation for + * pg_atomic_exchange. We assume the availability of 32-bit + * __atomic_compare_exchange_n() implies the availability of 32-bit + * __atomic_exchange_n(). + */ +#if !defined(PG_HAVE_ATOMIC_EXCHANGE_U32) && defined(HAVE_GCC__ATOMIC_INT32_CAS) +#define PG_HAVE_ATOMIC_EXCHANGE_U32 +static inline uint32 +pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 newval) +{ + return __atomic_exchange_n(&ptr->value, newval, __ATOMIC_SEQ_CST); +} +#endif + /* if we have 32-bit __sync_val_compare_and_swap, assume we have these too: */ #if !defined(PG_HAVE_ATOMIC_FETCH_ADD_U32) && defined(HAVE_GCC__SYNC_INT32_CAS) @@ -243,6 +259,22 @@ pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, } #endif +/* + * __sync_lock_test_and_set() only supports setting the value to 1 on some + * platforms, so we only provide an __atomic implementation for + * pg_atomic_exchange. We assume the availability of 64-bit + * __atomic_compare_exchange_n() implies the availability of 64-bit + * __atomic_exchange_n(). + */ +#if !defined(PG_HAVE_ATOMIC_EXCHANGE_U64) && defined(HAVE_GCC__ATOMIC_INT64_CAS) +#define PG_HAVE_ATOMIC_EXCHANGE_U64 +static inline uint64 +pg_atomic_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 newval) +{ + return __atomic_exchange_n(&ptr->value, newval, __ATOMIC_SEQ_CST); +} +#endif + /* if we have 64-bit __sync_val_compare_and_swap, assume we have these too: */ #if !defined(PG_HAVE_ATOMIC_FETCH_ADD_U64) && defined(HAVE_GCC__SYNC_INT64_CAS) diff --git a/src/include/port/atomics/generic-msvc.h b/src/include/port/atomics/generic-msvc.h index 8835f4ceea..7566d6f937 100644 --- a/src/include/port/atomics/generic-msvc.h +++ b/src/include/port/atomics/generic-msvc.h @@ -58,6 +58,13 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, return ret; } +#define PG_HAVE_ATOMIC_EXCHANGE_U32 +static inline uint32 +pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 newval) +{ + return InterlockedExchange(&ptr->value, newval); +} + #define PG_HAVE_ATOMIC_FETCH_ADD_U32 static inline uint32 pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_) @@ -88,6 +95,16 @@ pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, /* Only implemented on 64bit builds */ #ifdef _WIN64 + +#pragma intrinsic(_InterlockedExchange64) + +#define PG_HAVE_ATOMIC_EXCHANGE_U64 +static inline uint64 +pg_atomic_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 newval) +{ + return _InterlockedExchange64(&ptr->value, newval); +} + #pragma intrinsic(_InterlockedExchangeAdd64) #define PG_HAVE_ATOMIC_FETCH_ADD_U64 @@ -96,6 +113,7 @@ pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_) { return _InterlockedExchangeAdd64(&ptr->value, add_); } + #endif /* _WIN64 */ #endif /* HAVE_ATOMICS */ diff --git a/src/include/port/atomics/generic-sunpro.h b/src/include/port/atomics/generic-sunpro.h index 30f7d8b536..5d41b73de0 100644 --- a/src/include/port/atomics/generic-sunpro.h +++ b/src/include/port/atomics/generic-sunpro.h @@ -87,6 +87,13 @@ pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, return ret; } +#define PG_HAVE_ATOMIC_EXCHANGE_U32 +static inline uint32 +pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 newval) +{ + return atomic_swap_32(&ptr->value, newval); +} + #define PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U64 static inline bool pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, @@ -101,6 +108,13 @@ pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, return ret; } +#define PG_HAVE_ATOMIC_EXCHANGE_U64 +static inline uint64 +pg_atomic_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 newval) +{ + return atomic_swap_64(&ptr->value, newval); +} + #endif /* HAVE_ATOMIC_H */ #endif /* defined(HAVE_ATOMICS) */ -- 2.25.1
