From 7d4bb04c0309734bf9443fa8185634e4530a4178 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Tue, 9 Jul 2024 17:08:30 -0400 Subject: [PATCH 1/7] Require C11 Error out if the compiler does not support C11. Also removes support for `__thread` since we now rely on C11 `_Thread_local`. Signed-off-by: Joseph Schuchart --- config/opal_setup_cc.m4 | 33 ++++----------------------------- opal/mca/threads/thread_usage.h | 10 ---------- 2 files changed, 4 insertions(+), 39 deletions(-) diff --git a/config/opal_setup_cc.m4 b/config/opal_setup_cc.m4 index 70b8c6f133a..394826223fb 100644 --- a/config/opal_setup_cc.m4 +++ b/config/opal_setup_cc.m4 @@ -170,7 +170,7 @@ AC_DEFUN([OPAL_SETUP_CC],[ AC_REQUIRE([_OPAL_PROG_CC]) AC_REQUIRE([AM_PROG_CC_C_O]) - OPAL_VAR_SCOPE_PUSH([opal_prog_cc_c11_helper__Thread_local_available opal_prog_cc_c11_helper_atomic_var_available opal_prog_cc_c11_helper__Atomic_available opal_prog_cc_c11_helper__static_assert_available opal_prog_cc_c11_helper__Generic_available opal_prog_cc__thread_available opal_prog_cc_c11_helper_atomic_fetch_xor_explicit_available opal_prog_cc_c11_helper_proper__Atomic_support_in_atomics]) + OPAL_VAR_SCOPE_PUSH([opal_prog_cc_c11_helper__Thread_local_available opal_prog_cc_c11_helper_atomic_var_available opal_prog_cc_c11_helper__Atomic_available opal_prog_cc_c11_helper__static_assert_available opal_prog_cc_c11_helper__Generic_available opal_prog_cc_c11_helper_atomic_fetch_xor_explicit_available opal_prog_cc_c11_helper_proper__Atomic_support_in_atomics]) OPAL_PROG_CC_C11 @@ -179,33 +179,11 @@ AC_DEFUN([OPAL_SETUP_CC],[ OPAL_C_COMPILER_VENDOR([opal_c_vendor]) if test $opal_cv_c11_supported = no ; then - # It is not currently an error if C11 support is not available. Uncomment the - # following lines and update the warning when we require a C11 compiler. - # AC_MSG_WARNING([Open MPI requires a C11 (or newer) compiler]) - # AC_MSG_ERROR([Aborting.]) - # From Open MPI 1.7 on we require a C99 compliant compiler - dnl with autoconf 2.70 AC_PROG_CC makes AC_PROG_CC_C99 obsolete - m4_version_prereq([2.70], - [], - [AC_PROG_CC_C99]) - # The result of AC_PROG_CC_C99 is stored in ac_cv_prog_cc_c99 - if test "x$ac_cv_prog_cc_c99" = xno ; then - AC_MSG_WARN([Open MPI requires a C99 (or newer) compiler. C11 is recommended.]) - AC_MSG_ERROR([Aborting.]) - fi - - # Get the correct result for C11 support flags now that the compiler flags have - # changed - OPAL_PROG_CC_C11_HELPER([], [], []) + # C11 is required + AC_MSG_WARN([Open MPI requires a C11 (or newer) compiler. C11 is required.]) + AC_MSG_ERROR([Aborting.]) fi - # Check if compiler support __thread - OPAL_CC_HELPER([if $CC $1 supports __thread], [opal_prog_cc__thread_available], - [],[[static __thread int foo = 1;++foo;]]) - - OPAL_CC_HELPER([if $CC $1 supports C11 _Thread_local], [opal_prog_cc_c11_helper__Thread_local_available], - [],[[static _Thread_local int foo = 1;++foo;]]) - dnl At this time Open MPI only needs thread local and the atomic convenience types for C11 support. These dnl will likely be required in the future. AC_DEFINE_UNQUOTED([OPAL_C_HAVE__THREAD_LOCAL], [$opal_prog_cc_c11_helper__Thread_local_available], @@ -223,9 +201,6 @@ AC_DEFUN([OPAL_SETUP_CC],[ AC_DEFINE_UNQUOTED([OPAL_C_HAVE__STATIC_ASSERT], [$opal_prog_cc_c11_helper__static_assert_available], [Whether C compiler support _Static_assert keyword]) - AC_DEFINE_UNQUOTED([OPAL_C_HAVE___THREAD], [$opal_prog_cc__thread_available], - [Whether C compiler supports __thread]) - # Check for standard headers, needed here because needed before # the types checks. This is only necessary for Autoconf < v2.70. m4_version_prereq([2.70], diff --git a/opal/mca/threads/thread_usage.h b/opal/mca/threads/thread_usage.h index 3eeca968c06..4e2fd75a7e1 100644 --- a/opal/mca/threads/thread_usage.h +++ b/opal/mca/threads/thread_usage.h @@ -250,17 +250,7 @@ OPAL_THREAD_DEFINE_ATOMIC_SWAP(int64_t, int64_t, 64) # define OPAL_ATOMIC_SWAP_64 opal_thread_swap_64 /* thread local storage */ -#if OPAL_C_HAVE__THREAD_LOCAL # define opal_thread_local _Thread_local # define OPAL_HAVE_THREAD_LOCAL 1 -#elif OPAL_C_HAVE___THREAD /* OPAL_C_HAVE__THREAD_LOCAL */ -# define opal_thread_local __thread -# define OPAL_HAVE_THREAD_LOCAL 1 -#endif /* OPAL_C_HAVE___THREAD */ - -#if !defined(OPAL_HAVE_THREAD_LOCAL) -# define OPAL_HAVE_THREAD_LOCAL 0 -#endif /* !defined(OPAL_HAVE_THREAD_LOCAL) */ - #endif /* OPAL_MCA_THREADS_THREAD_USAGE_H */ From ca65d289b2dd2bb0e17111ea34869d417fd259c2 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Tue, 9 Jul 2024 17:54:07 -0400 Subject: [PATCH 2/7] Replace typeof() with size_t in loop header All uses of the `PREPARE_REQUESTS_WITH_NO_FREE` macro map to size_t. Signed-off-by: Joseph Schuchart --- ompi/mca/vprotocol/pessimist/vprotocol_pessimist_wait.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ompi/mca/vprotocol/pessimist/vprotocol_pessimist_wait.c b/ompi/mca/vprotocol/pessimist/vprotocol_pessimist_wait.c index 6da65f0cc7b..4d8481d883d 100644 --- a/ompi/mca/vprotocol/pessimist/vprotocol_pessimist_wait.c +++ b/ompi/mca/vprotocol/pessimist/vprotocol_pessimist_wait.c @@ -20,7 +20,7 @@ static int vprotocol_pessimist_request_no_free(ompi_request_t **req) { } #define PREPARE_REQUESTS_WITH_NO_FREE(count, requests) do { \ - for (typeof(count) i = 0; i < count; i++) \ + for (size_t i = 0; i < count; i++) \ { \ if(requests[i] == MPI_REQUEST_NULL) continue; \ requests[i]->req_free = vprotocol_pessimist_request_no_free; \ From 288868ef3790deb17c9fa154a09bbb941bb55713 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Wed, 10 Jul 2024 08:59:28 -0400 Subject: [PATCH 3/7] Add feature test macro _POSIX_C_SOURCE to enable fileno() Flex prior to 2.6.6 uses fileno() but does not define the needed feature test macros. See https://github.com/westes/flex/issues/263 for details. Signed-off-by: Joseph Schuchart --- opal/util/Makefile.am | 6 ++++++ opal/util/keyval/Makefile.am | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/opal/util/Makefile.am b/opal/util/Makefile.am index 3a835bc8fcb..5888146cc09 100644 --- a/opal/util/Makefile.am +++ b/opal/util/Makefile.am @@ -138,6 +138,12 @@ libopalutil_core_la_DEPENDENCIES = \ json/libopalutil_json.la \ keyval/libopalutilkeyval.la +# flex prior to version 2.6.6 uses the POSIX extension fileno() +# without providing the proper feature test macro, so +# we add the _POSIX_C_SOURCE macro here. +# See https://github.com/westes/flex/issues/263 +libopalutil_core_la_CFLAGS = -D_POSIX_C_SOURCE=200809L + # Conditionally install the header files if WANT_INSTALL_HEADERS diff --git a/opal/util/keyval/Makefile.am b/opal/util/keyval/Makefile.am index b00083d5d80..438c39121d4 100644 --- a/opal/util/keyval/Makefile.am +++ b/opal/util/keyval/Makefile.am @@ -30,5 +30,11 @@ libopalutilkeyval_la_SOURCES = \ keyval_lex.h \ keyval_lex.l +# flex prior to version 2.6.6 uses the POSIX extension fileno() +# without providing the proper feature test macro, so +# we add the _POSIX_C_SOURCE macro here. +# See https://github.com/westes/flex/issues/263 +libopalutilkeyval_la_CFLAGS = -D_POSIX_C_SOURCE=200809L + maintainer-clean-local: rm -f keyval_lex.c From 1015ddf9c7e0946b380eef30307d1371345f04c8 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Wed, 10 Jul 2024 09:17:07 -0400 Subject: [PATCH 4/7] Add feature test macro to enable pthread_mutexattr_settype() Signed-off-by: Joseph Schuchart --- .../pthreads/threads_pthreads_module.c | 40 +++++++++++++++++++ .../threads/pthreads/threads_pthreads_mutex.h | 39 +++--------------- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/opal/mca/threads/pthreads/threads_pthreads_module.c b/opal/mca/threads/pthreads/threads_pthreads_module.c index ac09b71d53d..bcbd1fded13 100644 --- a/opal/mca/threads/pthreads/threads_pthreads_module.c +++ b/opal/mca/threads/pthreads/threads_pthreads_module.c @@ -23,7 +23,11 @@ * $HEADER$ */ +/* needed for pthread_mutexattr_settype */ +#define _GNU_SOURCE + #include +#include #include "opal/constants.h" #include "opal/mca/threads/threads.h" @@ -38,3 +42,39 @@ int opal_tsd_key_create(opal_tsd_key_t *key, opal_tsd_destructor_t destructor) rc = pthread_key_create(key, destructor); return 0 == rc ? OPAL_SUCCESS : OPAL_ERR_IN_ERRNO; } + + +int opal_thread_internal_mutex_init_recursive(opal_thread_internal_mutex_t *p_mutex) +{ + int ret; +#if OPAL_ENABLE_DEBUG + pthread_mutexattr_t mutex_attr; + ret = pthread_mutexattr_init(&mutex_attr); + if (0 != ret) + return OPAL_ERR_IN_ERRNO; + ret = pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE); + if (0 != ret) { + ret = pthread_mutexattr_destroy(&mutex_attr); + assert(0 == ret); + return OPAL_ERR_IN_ERRNO; + } + ret = pthread_mutex_init(p_mutex, &mutex_attr); + if (0 != ret) { + ret = pthread_mutexattr_destroy(&mutex_attr); + assert(0 == ret); + return OPAL_ERR_IN_ERRNO; + } + ret = pthread_mutexattr_destroy(&mutex_attr); + assert(0 == ret); +#else + pthread_mutexattr_t mutex_attr; + ret = pthread_mutexattr_init(&mutex_attr); + if (0 != ret) { + return OPAL_ERR_IN_ERRNO; + } + pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE); + ret = pthread_mutex_init(p_mutex, &mutex_attr); + pthread_mutexattr_destroy(&mutex_attr); +#endif + return 0 == ret ? OPAL_SUCCESS : OPAL_ERR_IN_ERRNO; +} diff --git a/opal/mca/threads/pthreads/threads_pthreads_mutex.h b/opal/mca/threads/pthreads/threads_pthreads_mutex.h index 0207681ee9f..ee9dda81f18 100644 --- a/opal/mca/threads/pthreads/threads_pthreads_mutex.h +++ b/opal/mca/threads/pthreads/threads_pthreads_mutex.h @@ -61,48 +61,19 @@ typedef pthread_mutex_t opal_thread_internal_mutex_t; # define OPAL_THREAD_INTERNAL_RECURSIVE_MUTEX_INITIALIZER PTHREAD_RECURSIVE_MUTEX_INITIALIZER #endif + +int opal_thread_internal_mutex_init_recursive(opal_thread_internal_mutex_t *p_mutex); + static inline int opal_thread_internal_mutex_init(opal_thread_internal_mutex_t *p_mutex, bool recursive) { int ret; -#if OPAL_ENABLE_DEBUG - if (recursive) { - pthread_mutexattr_t mutex_attr; - ret = pthread_mutexattr_init(&mutex_attr); - if (0 != ret) - return OPAL_ERR_IN_ERRNO; - ret = pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE); - if (0 != ret) { - ret = pthread_mutexattr_destroy(&mutex_attr); - assert(0 == ret); - return OPAL_ERR_IN_ERRNO; - } - ret = pthread_mutex_init(p_mutex, &mutex_attr); - if (0 != ret) { - ret = pthread_mutexattr_destroy(&mutex_attr); - assert(0 == ret); - return OPAL_ERR_IN_ERRNO; - } - ret = pthread_mutexattr_destroy(&mutex_attr); - assert(0 == ret); - } else { - ret = pthread_mutex_init(p_mutex, NULL); - } -#else if (recursive) { - pthread_mutexattr_t mutex_attr; - ret = pthread_mutexattr_init(&mutex_attr); - if (0 != ret) { - return OPAL_ERR_IN_ERRNO; - } - pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE); - ret = pthread_mutex_init(p_mutex, &mutex_attr); - pthread_mutexattr_destroy(&mutex_attr); + return opal_thread_internal_mutex_init_recursive(p_mutex); } else { ret = pthread_mutex_init(p_mutex, NULL); + return 0 == ret ? OPAL_SUCCESS : OPAL_ERR_IN_ERRNO; } -#endif - return 0 == ret ? OPAL_SUCCESS : OPAL_ERR_IN_ERRNO; } static inline void opal_thread_internal_mutex_lock(opal_thread_internal_mutex_t *p_mutex) From 039cb58f598971e525e5366424c2d8c69291443e Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Wed, 10 Jul 2024 18:34:22 -0400 Subject: [PATCH 5/7] Enable nanosleep with _POSIX_C_SOURCE Move calls to nanosleep out of headers and add the feature test macro _POSIX_C_SOURCE to enable its use. This should not cause any significant overheads. Signed-off-by: Joseph Schuchart --- opal/class/opal_fifo.h | 2 +- opal/class/opal_lifo.c | 17 +++++++++++++++++ opal/class/opal_lifo.h | 15 ++------------- .../threads/pthreads/threads_pthreads_yield.c | 3 +++ 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/opal/class/opal_fifo.h b/opal/class/opal_fifo.h index 3f04f18ea71..816b2fb0ecd 100644 --- a/opal/class/opal_fifo.h +++ b/opal/class/opal_fifo.h @@ -224,7 +224,7 @@ static inline opal_list_item_t *opal_fifo_pop_atomic(opal_fifo_t *fifo) if (++attempt == 5) { /* deliberately suspend this thread to allow other threads to run. this should * only occur during periods of contention on the lifo. */ - _opal_lifo_release_cpu(); + opal_lifo_release_cpu(); attempt = 0; } diff --git a/opal/class/opal_lifo.c b/opal/class/opal_lifo.c index e4bcce794d8..0af332e3e3a 100644 --- a/opal/class/opal_lifo.c +++ b/opal/class/opal_lifo.c @@ -19,7 +19,11 @@ * $HEADER$ */ +/* needed for nanosleep() */ +#define _POSIX_C_SOURCE 200809L + #include "opal_config.h" +#include #include "opal/class/opal_lifo.h" static void opal_lifo_construct(opal_lifo_t *lifo) @@ -31,3 +35,16 @@ static void opal_lifo_construct(opal_lifo_t *lifo) } OBJ_CLASS_INSTANCE(opal_lifo_t, opal_object_t, opal_lifo_construct, NULL); + + +void opal_lifo_release_cpu(void) +{ + /* NTH: there are many ways to cause the current thread to be suspended. This one + * should work well in most cases. Another approach would be to use poll (NULL, 0, ) but + * the interval will be forced to be in ms (instead of ns or us). Note that there + * is a performance improvement for the lifo test when this call is made on detection + * of contention but it may not translate into actually MPI or application performance + * improvements. */ + static struct timespec interval = {.tv_sec = 0, .tv_nsec = 100}; + nanosleep(&interval, NULL); +} diff --git a/opal/class/opal_lifo.h b/opal/class/opal_lifo.h index d659ca2a9c7..72cc531e3b8 100644 --- a/opal/class/opal_lifo.h +++ b/opal/class/opal_lifo.h @@ -30,7 +30,6 @@ #include "opal_config.h" #include "opal/class/opal_list.h" -#include #include "opal/mca/threads/mutex.h" #include "opal/sys/atomic.h" @@ -92,17 +91,7 @@ opal_read_counted_pointer(volatile opal_counted_pointer_t *volatile addr, /** * @brief Helper function for lifo/fifo to sleep this thread if excessive contention is detected */ -static inline void _opal_lifo_release_cpu(void) -{ - /* NTH: there are many ways to cause the current thread to be suspended. This one - * should work well in most cases. Another approach would be to use poll (NULL, 0, ) but - * the interval will be forced to be in ms (instead of ns or us). Note that there - * is a performance improvement for the lifo test when this call is made on detection - * of contention but it may not translate into actually MPI or application performance - * improvements. */ - static struct timespec interval = {.tv_sec = 0, .tv_nsec = 100}; - nanosleep(&interval, NULL); -} +void opal_lifo_release_cpu(void); /* Atomic Last In First Out lists. If we are in a multi-threaded environment then the * atomicity is insured via the compare-and-swap operation, if not we simply do a read @@ -225,7 +214,7 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo) if (++attempt == 5) { /* deliberately suspend this thread to allow other threads to run. this should * only occur during periods of contention on the lifo. */ - _opal_lifo_release_cpu(); + opal_lifo_release_cpu(); attempt = 0; } diff --git a/opal/mca/threads/pthreads/threads_pthreads_yield.c b/opal/mca/threads/pthreads/threads_pthreads_yield.c index 3ae102e43e0..4ab5caa85ec 100644 --- a/opal/mca/threads/pthreads/threads_pthreads_yield.c +++ b/opal/mca/threads/pthreads/threads_pthreads_yield.c @@ -10,6 +10,9 @@ * $HEADER$ */ +/* needed for nanosleep() */ +#define _POSIX_C_SOURCE 200809L + #include "opal_config.h" #include #ifdef HAVE_SCHED_H From fe06e4859c44482eef3063a7da1949a66213fbc2 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Thu, 11 Jul 2024 09:43:00 -0400 Subject: [PATCH 6/7] Reduce_local test: Add feature test macros Needed for strsep(), posix_memalign(), and getopt(). Also fix the min/max macros. There is no need to cache the variables as these macros are used without producing side-effects. Signed-off-by: Joseph Schuchart --- test/datatype/reduce_local.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/test/datatype/reduce_local.c b/test/datatype/reduce_local.c index 9a428f35670..baa14310c50 100644 --- a/test/datatype/reduce_local.c +++ b/test/datatype/reduce_local.c @@ -5,6 +5,7 @@ * reserved. * Copyright (c) 2020 Research Organization for Information Science * and Technology (RIST). All rights reserved. + * Copyright (c) 2024 Stony Brook University. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -12,6 +13,13 @@ * $HEADER$ */ +/* needed for strsep() */ +#define _DEFAULT_SOURCE +#define _BSD_SOURCE + +/* needed for posix_memalign() and getopt() */ +#define _POSIX_C_SOURCE 200809L + #include #include #include @@ -50,19 +58,9 @@ static int do_ops[12] = { static int verbose = 0; static int total_errors = 0; -#define max(a, b) \ - ({ \ - __typeof__(a) _a = (a); \ - __typeof__(b) _b = (b); \ - _a > _b ? _a : _b; \ - }) - -#define min(a, b) \ - ({ \ - __typeof__(a) _a = (a); \ - __typeof__(b) _b = (b); \ - _a < _b ? _a : _b; \ - }) +#define max(a, b) (a) > (b) ? (a) : (b) + +#define min(a, b) (a) < (b) ? (a) : (b) static void print_status(char *op, char *type, int type_size, int count, int max_shift, double *duration, int repeats, int correct) From b21fc7a0bb0392bfd7e6e34b8feec213800191a6 Mon Sep 17 00:00:00 2001 From: Joseph Schuchart Date: Wed, 9 Oct 2024 09:13:29 -0400 Subject: [PATCH 7/7] Include ompi_config.h in reduce_local.c Signed-off-by: Joseph Schuchart --- test/datatype/reduce_local.c | 1 + 1 file changed, 1 insertion(+) diff --git a/test/datatype/reduce_local.c b/test/datatype/reduce_local.c index baa14310c50..9f1c06d4ba0 100644 --- a/test/datatype/reduce_local.c +++ b/test/datatype/reduce_local.c @@ -20,6 +20,7 @@ /* needed for posix_memalign() and getopt() */ #define _POSIX_C_SOURCE 200809L +#include "ompi_config.h" #include #include #include