Remove --disable-atomics, require 32 bit atomics.

Modern versions of all relevant architectures and tool chains have
atomics support.  Since edadeb0, 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 <stdatomic.h> 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 <hlinnaka@iki.fi>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (concept, not the patch)
Reviewed-by: Andres Freund <andres@anarazel.de> (concept, not the patch)
Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us

Author: macdice

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

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'], [],

doc/src/sgml/installation.sgml

@@ -1258,18 +1258,6 @@ build-postgresql:
        </listitem>
       </varlistentry>
 
-      <varlistentry id="configure-option-disable-atomics">
-       <term><option>--disable-atomics</option></term>
-       <listitem>
-        <para>
-         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.
-        </para>
-       </listitem>
-      </varlistentry>
-
      </variablelist>
 
    </sect3>
@@ -2674,19 +2662,6 @@ ninja install
       </listitem>
      </varlistentry>
 
-     <varlistentry id="configure-atomics-meson">
-      <term><option>-Datomics={ true | false }</option></term>
-      <listitem>
-       <para>
-        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.
-       </para>
-      </listitem>
-     </varlistentry>
-
     </variablelist>
    </sect3>
 

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
 
 
 ###############################################################

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

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 <atomic.h> header file. */
 #undef HAVE_ATOMIC_H
 

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"