Skip to content

Commit 447e06d

Browse files
committed
opal/atomic: always use C11 atomics if available
This commit disables the use of both the builtin and hand-written atomics if proper C11 atomic support is detected. This is the first step towards requiring the availability of C11 atomics for the C compiler used to build Open MPI. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent c09f046 commit 447e06d

File tree

12 files changed

+456
-25
lines changed

12 files changed

+456
-25
lines changed

config/opal_config_asm.m4

Lines changed: 137 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ dnl Copyright (c) 2008-2018 Cisco Systems, Inc. All rights reserved.
1313
dnl Copyright (c) 2010 Oracle and/or its affiliates. All rights reserved.
1414
dnl Copyright (c) 2015-2017 Research Organization for Information Science
1515
dnl and Technology (RIST). All rights reserved.
16-
dnl Copyright (c) 2014-2017 Los Alamos National Security, LLC. All rights
16+
dnl Copyright (c) 2014-2018 Los Alamos National Security, LLC. All rights
1717
dnl reserved.
1818
dnl Copyright (c) 2017 Amazon.com, Inc. or its affiliates. All Rights
1919
dnl reserved.
@@ -122,6 +122,58 @@ int main(int argc, char** argv)
122122
}
123123
]])
124124

125+
dnl This is a C test to see if 128-bit __atomic_compare_exchange_n()
126+
dnl actually works (e.g., it compiles and links successfully on
127+
dnl ARM64+clang, but returns incorrect answers as of August 2018).
128+
AC_DEFUN([OPAL_ATOMIC_COMPARE_EXCHANGE_STRONG_TEST_SOURCE],[[
129+
#include <stdint.h>
130+
#include <stdbool.h>
131+
#include <stdlib.h>
132+
#include <stdatomic.h>
133+
134+
typedef union {
135+
uint64_t fake@<:@2@:>@;
136+
_Atomic __int128 real;
137+
} ompi128;
138+
139+
static void test1(void)
140+
{
141+
// As of Aug 2018, we could not figure out a way to assign 128-bit
142+
// constants -- the compilers would not accept it. So use a fake
143+
// union to assign 2 uin64_t's to make a single __int128.
144+
ompi128 ptr = { .fake = { 0xFFEEDDCCBBAA0099, 0x8877665544332211 }};
145+
ompi128 expected = { .fake = { 0x11EEDDCCBBAA0099, 0x88776655443322FF }};
146+
ompi128 desired = { .fake = { 0x1122DDCCBBAA0099, 0x887766554433EEFF }};
147+
bool r = atomic_compare_exchange_strong (&ptr.real, &expected.real,
148+
desired.real, true,
149+
atomic_relaxed, atomic_relaxed);
150+
if ( !(r == false && ptr.real == expected.real)) {
151+
exit(1);
152+
}
153+
}
154+
155+
static void test2(void)
156+
{
157+
ompi128 ptr = { .fake = { 0xFFEEDDCCBBAA0099, 0x8877665544332211 }};
158+
ompi128 expected = ptr;
159+
ompi128 desired = { .fake = { 0x1122DDCCBBAA0099, 0x887766554433EEFF }};
160+
bool r = atomic_compare_exchange_strong (&ptr.real, &expected.real,
161+
desired.real, true,
162+
atomic_relaxed, atomic_relaxed);
163+
if (!(r == true && ptr.real == desired.real)) {
164+
exit(2);
165+
}
166+
}
167+
168+
vvvvvvvvvvvvvvvvvvvv
169+
int main(int argc, char** argv)
170+
{
171+
test1();
172+
test2();
173+
return 0;
174+
}
175+
]])
176+
125177
dnl ------------------------------------------------------------------
126178

127179
dnl
@@ -329,6 +381,71 @@ __atomic_add_fetch(&tmp64, 1, __ATOMIC_RELAXED);],
329381
OPAL_CHECK_GCC_BUILTIN_CSWAP_INT128
330382
])
331383

384+
AC_DEFUN([OPAL_CHECK_C11_CSWAP_INT128], [
385+
OPAL_VAR_SCOPE_PUSH([atomic_compare_exchange_result atomic_compare_exchange_CFLAGS_save atomic_compare_exchange_LIBS_save])
386+
387+
atomic_compare_exchange_CFLAGS_save=$CFLAGS
388+
atomic_compare_exchange_LIBS_save=$LIBS
389+
390+
# Do we have C11 atomics on 128-bit integers?
391+
# Use a special macro because we need to check with a few different
392+
# CFLAGS/LIBS.
393+
OPAL_ASM_CHECK_ATOMIC_FUNC([atomic_compare_exchange_strong_16],
394+
[AC_LANG_SOURCE(OPAL_ATOMIC_COMPARE_EXCHANGE_STRONG_TEST_SOURCE)],
395+
[atomic_compare_exchange_result=1],
396+
[atomic_compare_exchange_result=0])
397+
398+
# If we have it and it works, check to make sure it is always lock
399+
# free.
400+
AS_IF([test $atomic_compare_exchange_result -eq 1],
401+
[AC_MSG_CHECKING([if C11 __int128 atomic compare-and-swap is always lock-free])
402+
AC_RUN_IFELSE([AC_LANG_PROGRAM([#include <stdatomic.h>], [_Atomic __int128_t x; if (!atomic_is_lock_free(&x)) { return 1; }])],
403+
[AC_MSG_RESULT([yes])],
404+
[atomic_compare_exchange_result=0
405+
# If this test fails, need to reset CFLAGS/LIBS (the
406+
# above tests atomically set CFLAGS/LIBS or not; this
407+
# test is running after the fact, so we have to undo
408+
# the side-effects of setting CFLAGS/LIBS if the above
409+
# tests passed).
410+
CFLAGS=$atomic_compare_exchange_CFLAGS_save
411+
LIBS=$atomic_compare_exchange_LIBS_save
412+
AC_MSG_RESULT([no])],
413+
[AC_MSG_RESULT([cannot test -- assume yes (cross compiling)])])
414+
])
415+
416+
AC_DEFINE_UNQUOTED([OPAL_HAVE_C11_CSWAP_INT128],
417+
[$atomic_compare_exchange_result],
418+
[Whether C11 atomic compare swap is both supported and lock-free on 128-bit values])
419+
420+
dnl If we could not find decent support for 128-bits atomic let's
421+
dnl try the GCC _sync
422+
AS_IF([test $atomic_compare_exchange_result -eq 0],
423+
[OPAL_CHECK_SYNC_BUILTIN_CSWAP_INT128])
424+
425+
OPAL_VAR_SCOPE_POP
426+
])
427+
428+
AC_DEFUN([OPAL_CHECK_GCC_ATOMIC_BUILTINS], [
429+
AC_MSG_CHECKING([for __atomic builtin atomics])
430+
431+
AC_TRY_LINK([
432+
#include <stdint.h>
433+
uint32_t tmp, old = 0;
434+
uint64_t tmp64, old64 = 0;], [
435+
__atomic_thread_fence(__ATOMIC_SEQ_CST);
436+
__atomic_compare_exchange_n(&tmp, &old, 1, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
437+
__atomic_add_fetch(&tmp, 1, __ATOMIC_RELAXED);
438+
__atomic_compare_exchange_n(&tmp64, &old64, 1, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
439+
__atomic_add_fetch(&tmp64, 1, __ATOMIC_RELAXED);],
440+
[AC_MSG_RESULT([yes])
441+
$1],
442+
[AC_MSG_RESULT([no])
443+
$2])
444+
445+
# Check for 128-bit support
446+
OPAL_CHECK_GCC_BUILTIN_CSWAP_INT128
447+
])
448+
332449

333450
dnl #################################################################
334451
dnl
@@ -1020,17 +1137,27 @@ AC_DEFUN([OPAL_CONFIG_ASM],[
10201137
AC_REQUIRE([OPAL_SETUP_CC])
10211138
AC_REQUIRE([AM_PROG_AS])
10221139
1140+
AC_ARG_ENABLE([c11-atomics],[AC_HELP_STRING([--enable-c11-atomics],
1141+
[Enable use of C11 atomics if available (default: enabled)])])
1142+
10231143
AC_ARG_ENABLE([builtin-atomics],
10241144
[AC_HELP_STRING([--enable-builtin-atomics],
1025-
[Enable use of __sync builtin atomics (default: enabled)])])
1026-
1027-
opal_cv_asm_builtin="BUILTIN_NO"
1028-
AS_IF([test "$opal_cv_asm_builtin" = "BUILTIN_NO" && test "$enable_builtin_atomics" != "no"],
1029-
[OPAL_CHECK_GCC_ATOMIC_BUILTINS([opal_cv_asm_builtin="BUILTIN_GCC"], [])])
1030-
AS_IF([test "$opal_cv_asm_builtin" = "BUILTIN_NO" && test "$enable_builtin_atomics" != "no"],
1031-
[OPAL_CHECK_SYNC_BUILTINS([opal_cv_asm_builtin="BUILTIN_SYNC"], [])])
1032-
AS_IF([test "$opal_cv_asm_builtin" = "BUILTIN_NO" && test "$enable_builtin_atomics" = "yes"],
1033-
[AC_MSG_ERROR([__sync builtin atomics requested but not found.])])
1145+
[Enable use of __sync builtin atomics (default: disabled)])])
1146+
1147+
OPAL_CHECK_C11_CSWAP_INT128
1148+
1149+
if test "x$enable_c11_atomics" != "xno" && test "$opal_cv_c11_supported" = "yes" ; then
1150+
opal_cv_asm_builtin="BUILTIN_C11"
1151+
OPAL_CHECK_C11_CSWAP_INT128
1152+
else
1153+
opal_cv_asm_builtin="BUILTIN_NO"
1154+
AS_IF([test "$opal_cv_asm_builtin" = "BUILTIN_NO" && test "$enable_builtin_atomics" = "yes"],
1155+
[OPAL_CHECK_GCC_ATOMIC_BUILTINS([opal_cv_asm_builtin="BUILTIN_GCC"], [])])
1156+
AS_IF([test "$opal_cv_asm_builtin" = "BUILTIN_NO" && test "$enable_builtin_atomics" = "yes"],
1157+
[OPAL_CHECK_SYNC_BUILTINS([opal_cv_asm_builtin="BUILTIN_SYNC"], [])])
1158+
AS_IF([test "$opal_cv_asm_builtin" = "BUILTIN_NO" && test "$enable_builtin_atomics" = "yes"],
1159+
[AC_MSG_ERROR([__sync builtin atomics requested but not found.])])
1160+
fi
10341161
10351162
OPAL_CHECK_ASM_PROC
10361163
OPAL_CHECK_ASM_TEXT

opal/class/opal_fifo.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,9 @@ static inline opal_list_item_t *opal_fifo_pop_atomic (opal_fifo_t *fifo)
155155
* update the head */
156156

157157
/* wait for next pointer to be updated by push */
158-
while (ghost == item->opal_list_next) {
158+
do {
159159
opal_atomic_rmb ();
160-
}
161-
162-
opal_atomic_rmb ();
160+
} while (ghost == item->opal_list_next);
163161

164162
/* update the head with the real next value. note that no other thread
165163
* will be attempting to update the head until after it has been updated

opal/class/opal_object.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ int opal_class_init_epoch = 1;
5555
/*
5656
* Local variables
5757
*/
58-
static opal_atomic_lock_t class_lock = { { OPAL_ATOMIC_LOCK_UNLOCKED } };
58+
static opal_atomic_lock_t class_lock = OPAL_ATOMIC_LOCK_INIT;
5959
static void** classes = NULL;
6060
static int num_classes = 0;
6161
static int max_classes = 0;

opal/include/opal/sys/Makefile.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# All rights reserved.
1212
# Copyright (c) 2010 Cisco Systems, Inc. All rights reserved.
1313
# Copyright (c) 2011 Sandia National Laboratories. All rights reserved.
14-
# Copyright (c) 2016 Los Alamos National Security, LLC. All rights
14+
# Copyright (c) 2016-2018 Los Alamos National Security, LLC. All rights
1515
# reserved.
1616
# Copyright (c) 2017 Research Organization for Information Science
1717
# and Technology (RIST). All rights reserved.
@@ -27,6 +27,7 @@
2727
headers += \
2828
opal/sys/architecture.h \
2929
opal/sys/atomic.h \
30+
opal/sys/atomic_stdc.h \
3031
opal/sys/atomic_impl.h \
3132
opal/sys/timer.h \
3233
opal/sys/cma.h

opal/include/opal/sys/architecture.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#define OPAL_BUILTIN_SYNC 0200
4848
#define OPAL_BUILTIN_GCC 0202
4949
#define OPAL_BUILTIN_NO 0203
50+
#define OPAL_BUILTIN_C11 0204
5051

5152
/* Formats */
5253
#define OPAL_DEFAULT 1000 /* standard for given architecture */

opal/include/opal/sys/atomic.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@
5858
#include "opal/sys/architecture.h"
5959
#include "opal_stdatomic.h"
6060

61+
#if OPAL_ASSEMBLY_BUILTIN == OPAL_BUILTIN_C11
62+
63+
#include "atomic_stdc.h"
64+
65+
#else /* !OPAL_C_HAVE__ATOMIC */
66+
6167
/* do some quick #define cleanup in cases where we are doing
6268
testing... */
6369
#ifdef OPAL_DISABLE_INLINE_ASM
@@ -147,6 +153,8 @@ enum {
147153
OPAL_ATOMIC_LOCK_LOCKED = 1
148154
};
149155

156+
#define OPAL_ATOMIC_LOCK_INIT {.u = {.lock = OPAL_ATOMIC_LOCK_UNLOCKED}}
157+
150158
/**********************************************************************
151159
*
152160
* Load the appropriate architecture files and set some reasonable
@@ -643,6 +651,8 @@ static inline intptr_t opal_atomic_fetch_sub_ptr( opal_atomic_intptr_t* addr, vo
643651
*/
644652
#include "opal/sys/atomic_impl.h"
645653

654+
#endif /* !OPAL_C_HAVE__ATOMIC */
655+
646656
END_C_DECLS
647657

648658
#endif /* OPAL_SYS_ATOMIC_H */

0 commit comments

Comments
 (0)