From 81385261362962deb9861b39b509aeffe213721d Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 30 Jul 2024 21:52:46 +1200 Subject: [PATCH] Remove --disable-atomics, require 32 bit atomics. Modern versions of all relevant architectures and tool chains have atomics support. Since edadeb07, there is no remaining reason to carry code that simulates atomic flags and uint32 imperfectly with spinlocks. 64 bit atomics are still emulated with spinlocks, if needed, for now. Any modern compiler capable of implementing C11 must have the underlying operations we need, though we don't require C11 yet. We detect certain compilers and architectures, so hypothetical new systems might need adjustments here. Reviewed-by: Heikki Linnakangas Reviewed-by: Tom Lane (concept, not the patch) Reviewed-by: Andres Freund (concept, not the patch) Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us --- configure | 40 -------- configure.ac | 13 --- doc/src/sgml/installation.sgml | 25 ----- meson.build | 65 ++++++------- src/backend/port/atomics.c | 109 ---------------------- src/include/pg_config.h.in | 3 - src/include/port/atomics.h | 18 ++-- src/include/port/atomics/arch-x86.h | 8 -- src/include/port/atomics/fallback.h | 87 +---------------- src/include/port/atomics/generic-gcc.h | 4 - src/include/port/atomics/generic-msvc.h | 4 - src/include/port/atomics/generic-sunpro.h | 8 -- 12 files changed, 39 insertions(+), 345 deletions(-) diff --git a/configure b/configure index f8deaa8d78..8f684f7945 100755 --- a/configure +++ b/configure @@ -836,7 +836,6 @@ enable_integer_datetimes enable_nls with_pgport enable_rpath -enable_atomics enable_debug enable_profiling enable_coverage @@ -1528,7 +1527,6 @@ Optional Features: enable Native Language Support --disable-rpath do not embed shared library search path in executables - --disable-atomics do not use atomic operations --enable-debug build with debugging symbols (-g) --enable-profiling build with profiling enabled --enable-coverage build with coverage testing instrumentation @@ -3264,33 +3262,6 @@ fi -# -# Atomic operations -# - - -# Check whether --enable-atomics was given. -if test "${enable_atomics+set}" = set; then : - enableval=$enable_atomics; - case $enableval in - yes) - : - ;; - no) - : - ;; - *) - as_fn_error $? "no argument expected for --enable-atomics option" "$LINENO" 5 - ;; - esac - -else - enable_atomics=yes - -fi - - - # # --enable-debug adds -g to compiler flags # @@ -12156,17 +12127,6 @@ fi fi -if test "$enable_atomics" = yes; then - -$as_echo "#define HAVE_ATOMICS 1" >>confdefs.h - -else - { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: -*** Not using atomic operations will cause poor performance." >&5 -$as_echo "$as_me: WARNING: -*** Not using atomic operations will cause poor performance." >&2;} -fi - if test "$with_gssapi" = yes ; then if test "$PORTNAME" != "win32"; then { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing gss_store_cred_into" >&5 diff --git a/configure.ac b/configure.ac index a72169f574..75b73532fe 100644 --- a/configure.ac +++ b/configure.ac @@ -186,12 +186,6 @@ PGAC_ARG_BOOL(enable, rpath, yes, [do not embed shared library search path in executables]) AC_SUBST(enable_rpath) -# -# Atomic operations -# -PGAC_ARG_BOOL(enable, atomics, yes, - [do not use atomic operations]) - # # --enable-debug adds -g to compiler flags # @@ -1290,13 +1284,6 @@ failure. It is possible the compiler isn't looking in the proper directory. Use --without-zlib to disable zlib support.])]) fi -if test "$enable_atomics" = yes; then - AC_DEFINE(HAVE_ATOMICS, 1, [Define to 1 if you want to use atomics if available.]) -else - AC_MSG_WARN([ -*** Not using atomic operations will cause poor performance.]) -fi - if test "$with_gssapi" = yes ; then if test "$PORTNAME" != "win32"; then AC_SEARCH_LIBS(gss_store_cred_into, [gssapi_krb5 gss 'gssapi -lkrb5 -lcrypto'], [], diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 3f19f272b1..4ab8ddba7c 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -1258,18 +1258,6 @@ build-postgresql: - - - - - Disable use of CPU atomic operations. This option does nothing on - platforms that lack such operations. On platforms that do have - them, this will result in poor performance. This option is only - useful for debugging or making performance comparisons. - - - - @@ -2674,19 +2662,6 @@ ninja install - - - - - This option is set to true by default; setting it to false will - disable use of CPU atomic operations. The option does nothing on - platforms that lack such operations. On platforms that do have - them, disabling atomics will result in poor performance. Changing - this option is only useful for debugging or making performance comparisons. - - - - diff --git a/meson.build b/meson.build index 6a0d538365..7de0371226 100644 --- a/meson.build +++ b/meson.build @@ -2089,70 +2089,61 @@ endif # Atomics ############################################################### -if not get_option('atomics') - warning('Not using atomics will cause poor performance') -else - # XXX: perhaps we should require some atomics support in this case these - # days? - cdata.set('HAVE_ATOMICS', 1) - - atomic_checks = [ - {'name': 'HAVE_GCC__SYNC_CHAR_TAS', - 'desc': '__sync_lock_test_and_set(char)', - 'test': ''' +atomic_checks = [ + {'name': 'HAVE_GCC__SYNC_CHAR_TAS', + 'desc': '__sync_lock_test_and_set(char)', + 'test': ''' char lock = 0; __sync_lock_test_and_set(&lock, 1); __sync_lock_release(&lock);'''}, - {'name': 'HAVE_GCC__SYNC_INT32_TAS', - 'desc': '__sync_lock_test_and_set(int32)', - 'test': ''' + {'name': 'HAVE_GCC__SYNC_INT32_TAS', + 'desc': '__sync_lock_test_and_set(int32)', + 'test': ''' int lock = 0; __sync_lock_test_and_set(&lock, 1); __sync_lock_release(&lock);'''}, - {'name': 'HAVE_GCC__SYNC_INT32_CAS', - 'desc': '__sync_val_compare_and_swap(int32)', - 'test': ''' + {'name': 'HAVE_GCC__SYNC_INT32_CAS', + 'desc': '__sync_val_compare_and_swap(int32)', + 'test': ''' int val = 0; __sync_val_compare_and_swap(&val, 0, 37);'''}, - {'name': 'HAVE_GCC__SYNC_INT64_CAS', - 'desc': '__sync_val_compare_and_swap(int64)', - 'test': ''' + {'name': 'HAVE_GCC__SYNC_INT64_CAS', + 'desc': '__sync_val_compare_and_swap(int64)', + 'test': ''' INT64 val = 0; __sync_val_compare_and_swap(&val, 0, 37);'''}, - {'name': 'HAVE_GCC__ATOMIC_INT32_CAS', - 'desc': ' __atomic_compare_exchange_n(int32)', - 'test': ''' + {'name': 'HAVE_GCC__ATOMIC_INT32_CAS', + 'desc': ' __atomic_compare_exchange_n(int32)', + 'test': ''' int val = 0; int expect = 0; __atomic_compare_exchange_n(&val, &expect, 37, 0, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);'''}, - {'name': 'HAVE_GCC__ATOMIC_INT64_CAS', - 'desc': ' __atomic_compare_exchange_n(int64)', - 'test': ''' + {'name': 'HAVE_GCC__ATOMIC_INT64_CAS', + 'desc': ' __atomic_compare_exchange_n(int64)', + 'test': ''' INT64 val = 0; INT64 expect = 0; __atomic_compare_exchange_n(&val, &expect, 37, 0, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);'''}, - ] +] - foreach check : atomic_checks - test = ''' +foreach check : atomic_checks + test = ''' int main(void) { @0@ }'''.format(check['test']) - cdata.set(check['name'], - cc.links(test, - name: check['desc'], - args: test_c_args + ['-DINT64=@0@'.format(cdata.get('PG_INT64_TYPE'))]) ? 1 : false - ) - endforeach - -endif + cdata.set(check['name'], + cc.links(test, + name: check['desc'], + args: test_c_args + ['-DINT64=@0@'.format(cdata.get('PG_INT64_TYPE'))]) ? 1 : false + ) +endforeach ############################################################### diff --git a/src/backend/port/atomics.c b/src/backend/port/atomics.c index cd7ede9672..6f1e014d0b 100644 --- a/src/backend/port/atomics.c +++ b/src/backend/port/atomics.c @@ -49,115 +49,6 @@ pg_extern_compiler_barrier(void) #endif -#ifdef PG_HAVE_ATOMIC_FLAG_SIMULATION - -void -pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr) -{ - StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t), - "size mismatch of atomic_flag vs slock_t"); - - SpinLockInit((slock_t *) &ptr->sema); - - ptr->value = false; -} - -bool -pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr) -{ - uint32 oldval; - - SpinLockAcquire((slock_t *) &ptr->sema); - oldval = ptr->value; - ptr->value = true; - SpinLockRelease((slock_t *) &ptr->sema); - - return oldval == 0; -} - -void -pg_atomic_clear_flag_impl(volatile pg_atomic_flag *ptr) -{ - SpinLockAcquire((slock_t *) &ptr->sema); - ptr->value = false; - SpinLockRelease((slock_t *) &ptr->sema); -} - -bool -pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr) -{ - return ptr->value == 0; -} - -#endif /* PG_HAVE_ATOMIC_FLAG_SIMULATION */ - -#ifdef PG_HAVE_ATOMIC_U32_SIMULATION -void -pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_) -{ - StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t), - "size mismatch of atomic_uint32 vs slock_t"); - - SpinLockInit((slock_t *) &ptr->sema); - ptr->value = val_; -} - -void -pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val) -{ - /* - * One might think that an unlocked write doesn't need to acquire the - * spinlock, but one would be wrong. Even an unlocked write has to cause a - * concurrent pg_atomic_compare_exchange_u32() (et al) to fail. - */ - SpinLockAcquire((slock_t *) &ptr->sema); - ptr->value = val; - SpinLockRelease((slock_t *) &ptr->sema); -} - -bool -pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, - uint32 *expected, uint32 newval) -{ - bool ret; - - /* - * Do atomic op under a spinlock. It might look like we could just skip - * the cmpxchg if the lock isn't available, but that'd just emulate a - * 'weak' compare and swap. I.e. one that allows spurious failures. Since - * several algorithms rely on a strong variant and that is efficiently - * implementable on most major architectures let's emulate it here as - * well. - */ - SpinLockAcquire((slock_t *) &ptr->sema); - - /* perform compare/exchange logic */ - ret = ptr->value == *expected; - *expected = ptr->value; - if (ret) - ptr->value = newval; - - /* and release lock */ - SpinLockRelease((slock_t *) &ptr->sema); - - return ret; -} - -uint32 -pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_) -{ - uint32 oldval; - - SpinLockAcquire((slock_t *) &ptr->sema); - oldval = ptr->value; - ptr->value += add_; - SpinLockRelease((slock_t *) &ptr->sema); - return oldval; -} - -#endif /* PG_HAVE_ATOMIC_U32_SIMULATION */ - - #ifdef PG_HAVE_ATOMIC_U64_SIMULATION void diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index e6c06f6102..0e9b108e66 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -57,9 +57,6 @@ /* Define to 1 if you have the `ASN1_STRING_get0_data' function. */ #undef HAVE_ASN1_STRING_GET0_DATA -/* Define to 1 if you want to use atomics if available. */ -#undef HAVE_ATOMICS - /* Define to 1 if you have the header file. */ #undef HAVE_ATOMIC_H diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h index 03134e3b7b..c2ce10a718 100644 --- a/src/include/port/atomics.h +++ b/src/include/port/atomics.h @@ -17,7 +17,7 @@ * There exist generic, hardware independent, implementations for several * compilers which might be sufficient, although possibly not optimal, for a * new platform. If no such generic implementation is available spinlocks will - * be used to implement the API. + * be used to implement the 64-bit parts of the API. * * Implement _u64 atomics if and only if your platform can use them * efficiently (and obviously correctly). @@ -91,17 +91,17 @@ #elif defined(__SUNPRO_C) && !defined(__GNUC__) #include "port/atomics/generic-sunpro.h" #else -/* - * Unsupported compiler, we'll likely use slower fallbacks... At least - * compiler barriers should really be provided. - */ +/* Unknown compiler. */ +#endif + +/* Fail if we couldn't find implementations of required facilities. */ +#if !defined(PG_HAVE_ATOMIC_U32_SUPPORT) +#error "could not find an implementation of pg_atomic_uint32" #endif /* - * Provide a full fallback of the pg_*_barrier(), pg_atomic**_flag and - * pg_atomic_* APIs for platforms without sufficient spinlock and/or atomics - * support. In the case of spinlock backed atomics the emulation is expected - * to be efficient, although less so than native atomics support. + * Provide a spinlock-based implementation of the 64 bit variants, if + * necessary. */ #include "port/atomics/fallback.h" diff --git a/src/include/port/atomics/arch-x86.h b/src/include/port/atomics/arch-x86.h index 2a8eca30fc..c12f8a6069 100644 --- a/src/include/port/atomics/arch-x86.h +++ b/src/include/port/atomics/arch-x86.h @@ -49,8 +49,6 @@ * nice to support older gcc's and the compare/exchange implementation here is * actually more efficient than the * __sync variant. */ -#if defined(HAVE_ATOMICS) - #if defined(__GNUC__) || defined(__INTEL_COMPILER) #define PG_HAVE_ATOMIC_FLAG_SUPPORT @@ -80,8 +78,6 @@ typedef struct pg_atomic_uint64 #endif /* defined(__GNUC__) || defined(__INTEL_COMPILER) */ -#endif /* defined(HAVE_ATOMICS) */ - #if !defined(PG_HAVE_SPIN_DELAY) /* * This sequence is equivalent to the PAUSE instruction ("rep" is @@ -132,8 +128,6 @@ pg_spin_delay_impl(void) #endif /* !defined(PG_HAVE_SPIN_DELAY) */ -#if defined(HAVE_ATOMICS) - #if defined(__GNUC__) || defined(__INTEL_COMPILER) #define PG_HAVE_ATOMIC_TEST_SET_FLAG @@ -250,5 +244,3 @@ pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_) defined(__x86_64__) || defined(__x86_64) || defined(_M_X64) /* gcc, sunpro, msvc */ #define PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY #endif /* 8 byte single-copy atomicity */ - -#endif /* HAVE_ATOMICS */ diff --git a/src/include/port/atomics/fallback.h b/src/include/port/atomics/fallback.h index 2e3eef4aca..8ffd1a8fd3 100644 --- a/src/include/port/atomics/fallback.h +++ b/src/include/port/atomics/fallback.h @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * fallback.h - * Fallback for platforms without spinlock and/or atomics support. Slower + * Fallback for platforms without 64 bit atomics support. Slower * than native atomics support, but not unusably slow. * * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group @@ -49,50 +49,6 @@ extern void pg_extern_compiler_barrier(void); #endif -/* - * If we have atomics implementation for this platform, fall back to providing - * the atomics API using a spinlock to protect the internal state. Possibly - * the spinlock implementation uses semaphores internally... - * - * We have to be a bit careful here, as it's not guaranteed that atomic - * variables are mapped to the same address in every process (e.g. dynamic - * shared memory segments). We can't just hash the address and use that to map - * to a spinlock. Instead assign a spinlock on initialization of the atomic - * variable. - */ -#if !defined(PG_HAVE_ATOMIC_FLAG_SUPPORT) && !defined(PG_HAVE_ATOMIC_U32_SUPPORT) - -#define PG_HAVE_ATOMIC_FLAG_SIMULATION -#define PG_HAVE_ATOMIC_FLAG_SUPPORT - -typedef struct pg_atomic_flag -{ - /* - * To avoid circular includes we can't use s_lock as a type here. Instead - * just reserve enough space for all spinlock types. Some platforms would - * be content with just one byte instead of 4, but that's not too much - * waste. - */ - int sema; - volatile bool value; -} pg_atomic_flag; - -#endif /* PG_HAVE_ATOMIC_FLAG_SUPPORT */ - -#if !defined(PG_HAVE_ATOMIC_U32_SUPPORT) - -#define PG_HAVE_ATOMIC_U32_SIMULATION - -#define PG_HAVE_ATOMIC_U32_SUPPORT -typedef struct pg_atomic_uint32 -{ - /* Check pg_atomic_flag's definition above for an explanation */ - int sema; - volatile uint32 value; -} pg_atomic_uint32; - -#endif /* PG_HAVE_ATOMIC_U32_SUPPORT */ - #if !defined(PG_HAVE_ATOMIC_U64_SUPPORT) #define PG_HAVE_ATOMIC_U64_SIMULATION @@ -100,49 +56,10 @@ typedef struct pg_atomic_uint32 #define PG_HAVE_ATOMIC_U64_SUPPORT typedef struct pg_atomic_uint64 { - /* Check pg_atomic_flag's definition above for an explanation */ int sema; volatile uint64 value; } pg_atomic_uint64; -#endif /* PG_HAVE_ATOMIC_U64_SUPPORT */ - -#ifdef PG_HAVE_ATOMIC_FLAG_SIMULATION - -#define PG_HAVE_ATOMIC_INIT_FLAG -extern void pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr); - -#define PG_HAVE_ATOMIC_TEST_SET_FLAG -extern bool pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr); - -#define PG_HAVE_ATOMIC_CLEAR_FLAG -extern void pg_atomic_clear_flag_impl(volatile pg_atomic_flag *ptr); - -#define PG_HAVE_ATOMIC_UNLOCKED_TEST_FLAG -extern bool pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr); - -#endif /* PG_HAVE_ATOMIC_FLAG_SIMULATION */ - -#ifdef PG_HAVE_ATOMIC_U32_SIMULATION - -#define PG_HAVE_ATOMIC_INIT_U32 -extern void pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_); - -#define PG_HAVE_ATOMIC_WRITE_U32 -extern void pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val); - -#define PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U32 -extern bool pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, - uint32 *expected, uint32 newval); - -#define PG_HAVE_ATOMIC_FETCH_ADD_U32 -extern uint32 pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_); - -#endif /* PG_HAVE_ATOMIC_U32_SIMULATION */ - - -#ifdef PG_HAVE_ATOMIC_U64_SIMULATION - #define PG_HAVE_ATOMIC_INIT_U64 extern void pg_atomic_init_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 val_); @@ -153,4 +70,4 @@ extern bool pg_atomic_compare_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, #define PG_HAVE_ATOMIC_FETCH_ADD_U64 extern uint64 pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_); -#endif /* PG_HAVE_ATOMIC_U64_SIMULATION */ +#endif /* PG_HAVE_ATOMIC_U64_SUPPORT */ diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h index 872d2f02af..cfbcbe0fff 100644 --- a/src/include/port/atomics/generic-gcc.h +++ b/src/include/port/atomics/generic-gcc.h @@ -53,8 +53,6 @@ #endif -#ifdef HAVE_ATOMICS - /* generic gcc based atomic flag implementation */ #if !defined(PG_HAVE_ATOMIC_FLAG_SUPPORT) \ && (defined(HAVE_GCC__SYNC_INT32_TAS) || defined(HAVE_GCC__SYNC_CHAR_TAS)) @@ -319,5 +317,3 @@ pg_atomic_fetch_or_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 or_) #endif #endif /* !defined(PG_DISABLE_64_BIT_ATOMICS) */ - -#endif /* defined(HAVE_ATOMICS) */ diff --git a/src/include/port/atomics/generic-msvc.h b/src/include/port/atomics/generic-msvc.h index c013aca5e7..677436f260 100644 --- a/src/include/port/atomics/generic-msvc.h +++ b/src/include/port/atomics/generic-msvc.h @@ -30,8 +30,6 @@ #define pg_memory_barrier_impl() MemoryBarrier() #endif -#if defined(HAVE_ATOMICS) - #define PG_HAVE_ATOMIC_U32_SUPPORT typedef struct pg_atomic_uint32 { @@ -115,5 +113,3 @@ pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 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 840a45e778..08f093ed2c 100644 --- a/src/include/port/atomics/generic-sunpro.h +++ b/src/include/port/atomics/generic-sunpro.h @@ -17,8 +17,6 @@ * ------------------------------------------------------------------------- */ -#if defined(HAVE_ATOMICS) - #ifdef HAVE_MBARRIER_H #include @@ -66,10 +64,6 @@ typedef struct pg_atomic_uint64 #endif /* HAVE_ATOMIC_H */ -#endif /* defined(HAVE_ATOMICS) */ - - -#if defined(HAVE_ATOMICS) #ifdef HAVE_ATOMIC_H @@ -117,5 +111,3 @@ pg_atomic_exchange_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 newval) } #endif /* HAVE_ATOMIC_H */ - -#endif /* defined(HAVE_ATOMICS) */