Skip to content

Commit 8138526

Browse files
macdicemacdice
committed
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
1 parent e256266 commit 8138526

File tree

12 files changed

+39
-345
lines changed

12 files changed

+39
-345
lines changed

‎configure

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -836,7 +836,6 @@ enable_integer_datetimes
836836
enable_nls
837837
with_pgport
838838
enable_rpath
839-
enable_atomics
840839
enable_debug
841840
enable_profiling
842841
enable_coverage
@@ -1528,7 +1527,6 @@ Optional Features:
15281527
enable Native Language Support
15291528
--disable-rpath do not embed shared library search path in
15301529
executables
1531-
--disable-atomics do not use atomic operations
15321530
--enable-debug build with debugging symbols (-g)
15331531
--enable-profiling build with profiling enabled
15341532
--enable-coverage build with coverage testing instrumentation
@@ -3264,33 +3262,6 @@ fi
32643262

32653263

32663264

3267-
#
3268-
# Atomic operations
3269-
#
3270-
3271-
3272-
# Check whether --enable-atomics was given.
3273-
if test "${enable_atomics+set}" = set; then :
3274-
enableval=$enable_atomics;
3275-
case $enableval in
3276-
yes)
3277-
:
3278-
;;
3279-
no)
3280-
:
3281-
;;
3282-
*)
3283-
as_fn_error $? "no argument expected for --enable-atomics option" "$LINENO" 5
3284-
;;
3285-
esac
3286-
3287-
else
3288-
enable_atomics=yes
3289-
3290-
fi
3291-
3292-
3293-
32943265
#
32953266
# --enable-debug adds -g to compiler flags
32963267
#
@@ -12156,17 +12127,6 @@ fi
1215612127

1215712128
fi
1215812129

12159-
if test "$enable_atomics" = yes; then
12160-
12161-
$as_echo "#define HAVE_ATOMICS 1" >>confdefs.h
12162-
12163-
else
12164-
{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING:
12165-
*** Not using atomic operations will cause poor performance." >&5
12166-
$as_echo "$as_me: WARNING:
12167-
*** Not using atomic operations will cause poor performance." >&2;}
12168-
fi
12169-
1217012130
if test "$with_gssapi" = yes ; then
1217112131
if test "$PORTNAME" != "win32"; then
1217212132
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing gss_store_cred_into" >&5

‎configure.ac

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,6 @@ PGAC_ARG_BOOL(enable, rpath, yes,
186186
[do not embed shared library search path in executables])
187187
AC_SUBST(enable_rpath)
188188

189-
#
190-
# Atomic operations
191-
#
192-
PGAC_ARG_BOOL(enable, atomics, yes,
193-
[do not use atomic operations])
194-
195189
#
196190
# --enable-debug adds -g to compiler flags
197191
#
@@ -1290,13 +1284,6 @@ failure. It is possible the compiler isn't looking in the proper directory.
12901284
Use --without-zlib to disable zlib support.])])
12911285
fi
12921286

1293-
if test "$enable_atomics" = yes; then
1294-
AC_DEFINE(HAVE_ATOMICS, 1, [Define to 1 if you want to use atomics if available.])
1295-
else
1296-
AC_MSG_WARN([
1297-
*** Not using atomic operations will cause poor performance.])
1298-
fi
1299-
13001287
if test "$with_gssapi" = yes ; then
13011288
if test "$PORTNAME" != "win32"; then
13021289
AC_SEARCH_LIBS(gss_store_cred_into, [gssapi_krb5 gss 'gssapi -lkrb5 -lcrypto'], [],

‎doc/src/sgml/installation.sgml

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,18 +1258,6 @@ build-postgresql:
12581258
</listitem>
12591259
</varlistentry>
12601260

1261-
<varlistentry id="configure-option-disable-atomics">
1262-
<term><option>--disable-atomics</option></term>
1263-
<listitem>
1264-
<para>
1265-
Disable use of CPU atomic operations. This option does nothing on
1266-
platforms that lack such operations. On platforms that do have
1267-
them, this will result in poor performance. This option is only
1268-
useful for debugging or making performance comparisons.
1269-
</para>
1270-
</listitem>
1271-
</varlistentry>
1272-
12731261
</variablelist>
12741262

12751263
</sect3>
@@ -2674,19 +2662,6 @@ ninja install
26742662
</listitem>
26752663
</varlistentry>
26762664

2677-
<varlistentry id="configure-atomics-meson">
2678-
<term><option>-Datomics={ true | false }</option></term>
2679-
<listitem>
2680-
<para>
2681-
This option is set to true by default; setting it to false will
2682-
disable use of CPU atomic operations. The option does nothing on
2683-
platforms that lack such operations. On platforms that do have
2684-
them, disabling atomics will result in poor performance. Changing
2685-
this option is only useful for debugging or making performance comparisons.
2686-
</para>
2687-
</listitem>
2688-
</varlistentry>
2689-
26902665
</variablelist>
26912666
</sect3>
26922667

‎meson.build

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2089,70 +2089,61 @@ endif
20892089
# Atomics
20902090
###############################################################
20912091

2092-
if not get_option('atomics')
2093-
warning('Not using atomics will cause poor performance')
2094-
else
2095-
# XXX: perhaps we should require some atomics support in this case these
2096-
# days?
2097-
cdata.set('HAVE_ATOMICS', 1)
2098-
2099-
atomic_checks = [
2100-
{'name': 'HAVE_GCC__SYNC_CHAR_TAS',
2101-
'desc': '__sync_lock_test_and_set(char)',
2102-
'test': '''
2092+
atomic_checks = [
2093+
{'name': 'HAVE_GCC__SYNC_CHAR_TAS',
2094+
'desc': '__sync_lock_test_and_set(char)',
2095+
'test': '''
21032096
char lock = 0;
21042097
__sync_lock_test_and_set(&lock, 1);
21052098
__sync_lock_release(&lock);'''},
21062099

2107-
{'name': 'HAVE_GCC__SYNC_INT32_TAS',
2108-
'desc': '__sync_lock_test_and_set(int32)',
2109-
'test': '''
2100+
{'name': 'HAVE_GCC__SYNC_INT32_TAS',
2101+
'desc': '__sync_lock_test_and_set(int32)',
2102+
'test': '''
21102103
int lock = 0;
21112104
__sync_lock_test_and_set(&lock, 1);
21122105
__sync_lock_release(&lock);'''},
21132106

2114-
{'name': 'HAVE_GCC__SYNC_INT32_CAS',
2115-
'desc': '__sync_val_compare_and_swap(int32)',
2116-
'test': '''
2107+
{'name': 'HAVE_GCC__SYNC_INT32_CAS',
2108+
'desc': '__sync_val_compare_and_swap(int32)',
2109+
'test': '''
21172110
int val = 0;
21182111
__sync_val_compare_and_swap(&val, 0, 37);'''},
21192112

2120-
{'name': 'HAVE_GCC__SYNC_INT64_CAS',
2121-
'desc': '__sync_val_compare_and_swap(int64)',
2122-
'test': '''
2113+
{'name': 'HAVE_GCC__SYNC_INT64_CAS',
2114+
'desc': '__sync_val_compare_and_swap(int64)',
2115+
'test': '''
21232116
INT64 val = 0;
21242117
__sync_val_compare_and_swap(&val, 0, 37);'''},
21252118

2126-
{'name': 'HAVE_GCC__ATOMIC_INT32_CAS',
2127-
'desc': ' __atomic_compare_exchange_n(int32)',
2128-
'test': '''
2119+
{'name': 'HAVE_GCC__ATOMIC_INT32_CAS',
2120+
'desc': ' __atomic_compare_exchange_n(int32)',
2121+
'test': '''
21292122
int val = 0;
21302123
int expect = 0;
21312124
__atomic_compare_exchange_n(&val, &expect, 37, 0, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);'''},
21322125

2133-
{'name': 'HAVE_GCC__ATOMIC_INT64_CAS',
2134-
'desc': ' __atomic_compare_exchange_n(int64)',
2135-
'test': '''
2126+
{'name': 'HAVE_GCC__ATOMIC_INT64_CAS',
2127+
'desc': ' __atomic_compare_exchange_n(int64)',
2128+
'test': '''
21362129
INT64 val = 0;
21372130
INT64 expect = 0;
21382131
__atomic_compare_exchange_n(&val, &expect, 37, 0, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED);'''},
2139-
]
2132+
]
21402133

2141-
foreach check : atomic_checks
2142-
test = '''
2134+
foreach check : atomic_checks
2135+
test = '''
21432136
int main(void)
21442137
{
21452138
@0@
21462139
}'''.format(check['test'])
21472140

2148-
cdata.set(check['name'],
2149-
cc.links(test,
2150-
name: check['desc'],
2151-
args: test_c_args + ['-DINT64=@0@'.format(cdata.get('PG_INT64_TYPE'))]) ? 1 : false
2152-
)
2153-
endforeach
2154-
2155-
endif
2141+
cdata.set(check['name'],
2142+
cc.links(test,
2143+
name: check['desc'],
2144+
args: test_c_args + ['-DINT64=@0@'.format(cdata.get('PG_INT64_TYPE'))]) ? 1 : false
2145+
)
2146+
endforeach
21562147

21572148

21582149
###############################################################

‎src/backend/port/atomics.c

Lines changed: 0 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -49,115 +49,6 @@ pg_extern_compiler_barrier(void)
4949
#endif
5050

5151

52-
#ifdef PG_HAVE_ATOMIC_FLAG_SIMULATION
53-
54-
void
55-
pg_atomic_init_flag_impl(volatile pg_atomic_flag *ptr)
56-
{
57-
StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t),
58-
"size mismatch of atomic_flag vs slock_t");
59-
60-
SpinLockInit((slock_t *) &ptr->sema);
61-
62-
ptr->value = false;
63-
}
64-
65-
bool
66-
pg_atomic_test_set_flag_impl(volatile pg_atomic_flag *ptr)
67-
{
68-
uint32 oldval;
69-
70-
SpinLockAcquire((slock_t *) &ptr->sema);
71-
oldval = ptr->value;
72-
ptr->value = true;
73-
SpinLockRelease((slock_t *) &ptr->sema);
74-
75-
return oldval == 0;
76-
}
77-
78-
void
79-
pg_atomic_clear_flag_impl(volatile pg_atomic_flag *ptr)
80-
{
81-
SpinLockAcquire((slock_t *) &ptr->sema);
82-
ptr->value = false;
83-
SpinLockRelease((slock_t *) &ptr->sema);
84-
}
85-
86-
bool
87-
pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr)
88-
{
89-
return ptr->value == 0;
90-
}
91-
92-
#endif /* PG_HAVE_ATOMIC_FLAG_SIMULATION */
93-
94-
#ifdef PG_HAVE_ATOMIC_U32_SIMULATION
95-
void
96-
pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_)
97-
{
98-
StaticAssertDecl(sizeof(ptr->sema) >= sizeof(slock_t),
99-
"size mismatch of atomic_uint32 vs slock_t");
100-
101-
SpinLockInit((slock_t *) &ptr->sema);
102-
ptr->value = val_;
103-
}
104-
105-
void
106-
pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val)
107-
{
108-
/*
109-
* One might think that an unlocked write doesn't need to acquire the
110-
* spinlock, but one would be wrong. Even an unlocked write has to cause a
111-
* concurrent pg_atomic_compare_exchange_u32() (et al) to fail.
112-
*/
113-
SpinLockAcquire((slock_t *) &ptr->sema);
114-
ptr->value = val;
115-
SpinLockRelease((slock_t *) &ptr->sema);
116-
}
117-
118-
bool
119-
pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
120-
uint32 *expected, uint32 newval)
121-
{
122-
bool ret;
123-
124-
/*
125-
* Do atomic op under a spinlock. It might look like we could just skip
126-
* the cmpxchg if the lock isn't available, but that'd just emulate a
127-
* 'weak' compare and swap. I.e. one that allows spurious failures. Since
128-
* several algorithms rely on a strong variant and that is efficiently
129-
* implementable on most major architectures let's emulate it here as
130-
* well.
131-
*/
132-
SpinLockAcquire((slock_t *) &ptr->sema);
133-
134-
/* perform compare/exchange logic */
135-
ret = ptr->value == *expected;
136-
*expected = ptr->value;
137-
if (ret)
138-
ptr->value = newval;
139-
140-
/* and release lock */
141-
SpinLockRelease((slock_t *) &ptr->sema);
142-
143-
return ret;
144-
}
145-
146-
uint32
147-
pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
148-
{
149-
uint32 oldval;
150-
151-
SpinLockAcquire((slock_t *) &ptr->sema);
152-
oldval = ptr->value;
153-
ptr->value += add_;
154-
SpinLockRelease((slock_t *) &ptr->sema);
155-
return oldval;
156-
}
157-
158-
#endif /* PG_HAVE_ATOMIC_U32_SIMULATION */
159-
160-
16152
#ifdef PG_HAVE_ATOMIC_U64_SIMULATION
16253

16354
void

‎src/include/pg_config.h.in

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@
5757
/* Define to 1 if you have the `ASN1_STRING_get0_data' function. */
5858
#undef HAVE_ASN1_STRING_GET0_DATA
5959

60-
/* Define to 1 if you want to use atomics if available. */
61-
#undef HAVE_ATOMICS
62-
6360
/* Define to 1 if you have the <atomic.h> header file. */
6461
#undef HAVE_ATOMIC_H
6562

‎src/include/port/atomics.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* There exist generic, hardware independent, implementations for several
1818
* compilers which might be sufficient, although possibly not optimal, for a
1919
* new platform. If no such generic implementation is available spinlocks will
20-
* be used to implement the API.
20+
* be used to implement the 64-bit parts of the API.
2121
*
2222
* Implement _u64 atomics if and only if your platform can use them
2323
* efficiently (and obviously correctly).
@@ -91,17 +91,17 @@
9191
#elif defined(__SUNPRO_C) && !defined(__GNUC__)
9292
#include "port/atomics/generic-sunpro.h"
9393
#else
94-
/*
95-
* Unsupported compiler, we'll likely use slower fallbacks... At least
96-
* compiler barriers should really be provided.
97-
*/
94+
/* Unknown compiler. */
95+
#endif
96+
97+
/* Fail if we couldn't find implementations of required facilities. */
98+
#if !defined(PG_HAVE_ATOMIC_U32_SUPPORT)
99+
#error "could not find an implementation of pg_atomic_uint32"
98100
#endif
99101

100102
/*
101-
* Provide a full fallback of the pg_*_barrier(), pg_atomic**_flag and
102-
* pg_atomic_* APIs for platforms without sufficient spinlock and/or atomics
103-
* support. In the case of spinlock backed atomics the emulation is expected
104-
* to be efficient, although less so than native atomics support.
103+
* Provide a spinlock-based implementation of the 64 bit variants, if
104+
* necessary.
105105
*/
106106
#include "port/atomics/fallback.h"
107107

0 commit comments

Comments
 (0)