From df95ecb66be7ecffdfbe2f0b54b77ef7b7184d36 Mon Sep 17 00:00:00 2001 From: Jose Santiago Date: Wed, 1 Sep 2021 13:50:36 -0500 Subject: [PATCH] [core] Fix atomic for MacOS, FreeBSD, GCC<4.7, Clang<6 (#2104) --- CMakeLists.txt | 7 ++ srtcore/atomic.h | 151 ++++++++++++++++++++++++++++++++++------- srtcore/atomic_clock.h | 91 +++++++++++++++++++++++++ srtcore/sync.h | 74 ++------------------ 4 files changed, 233 insertions(+), 90 deletions(-) create mode 100644 srtcore/atomic_clock.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 3693b5b37..c5cfa1394 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -149,6 +149,13 @@ option(ENABLE_SOCK_CLOEXEC "Enable setting SOCK_CLOEXEC on a socket" ON) option(ENABLE_CLANG_TSA "Enable Clang Thread Safety Analysis" OFF) +# NOTE: Use ATOMIC_USE_SRT_SYNC_MUTEX and will override the auto-detection of the +# Atomic implemetation in srtcore/atomic.h. +option(ATOMIC_USE_SRT_SYNC_MUTEX "Use srt::sync::Mutex to Implement Atomics" OFF) +if (ATOMIC_USE_SRT_SYNC_MUTEX) + add_definitions(-DATOMIC_USE_SRT_SYNC_MUTEX=1) +endif() + set(TARGET_srt "srt" CACHE STRING "The name for the SRT library") # Use application-defined group reader diff --git a/srtcore/atomic.h b/srtcore/atomic.h index 5cedd396d..caec84fe1 100644 --- a/srtcore/atomic.h +++ b/srtcore/atomic.h @@ -61,16 +61,54 @@ line)[(2 * static_cast(!!(condition))) - 1] _impl_UNUSED #endif -#if defined(__GNUC__) || defined(__clang__) || defined(__xlc__) -#define ATOMIC_USE_GCC_INTRINSICS +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + // NOTE: Defined at the top level. +#elif defined(__APPLE__) && (__cplusplus >= 201103L) + // NOTE: Does support c++11 std::atomic, but the compiler may or + // may not support GCC atomic intrinsics. So go ahead and use the + // std::atomic implementation. + #define ATOMIC_USE_CPP11_ATOMIC +#elif (defined(__clang__) && defined(__clang_major__) && (__clang_major__ > 5)) \ + || defined(__xlc__) + // NOTE: Clang <6 does not support GCC __atomic_* intrinsics. I am unsure + // about Clang6. Since Clang sets __GNUC__ and __GNUC_MINOR__ of this era + // to <4.5, older Clang will catch the setting below to use the + // POSIX Mutex Implementation. + #define ATOMIC_USE_GCC_INTRINSICS +#elif defined(__GNUC__) \ + && ( (__GNUC__ > 4) || ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 7)) ) + // NOTE: The __atomic_* family of intrisics were introduced in GCC-4.7.0. + // NOTE: This follows #if defined(__clang__), because most if, not all, + // versions of Clang define __GNUC__ and __GNUC_MINOR__ but often define + // them to 4.4 or an even earlier version. Most of the newish versions + // of Clang also support GCC Atomic Intrisics even if they set GCC version + // macros to <4.7. + #define ATOMIC_USE_GCC_INTRINSICS +#elif defined(__GNUC__) && !defined(ATOMIC_USE_SRT_SYNC_MUTEX) + // NOTE: GCC compiler built-ins for atomic operations are pure + // compiler extensions prior to GCC-4.7 and were grouped into the + // the __sync_* family of functions. GCC-4.7, both the c++11 and C11 + // standards had been finalized, and GCC updated their built-ins to + // better reflect the new memory model and the new functions grouped + // into the __atomic_* family. Also the memory models were defined + // differently, than in pre 4.7. + // TODO: PORT to the pre GCC-4.7 __sync_* intrinsics. In the meantime use + // the POSIX Mutex Implementation. + #define ATOMIC_USE_SRT_SYNC_MUTEX 1 #elif defined(_MSC_VER) -#define ATOMIC_USE_MSVC_INTRINSICS -#include "atomic_msvc.h" + #define ATOMIC_USE_MSVC_INTRINSICS + #include "atomic_msvc.h" #elif __cplusplus >= 201103L -#define ATOMIC_USE_CPP11_ATOMIC -#include + #define ATOMIC_USE_CPP11_ATOMIC #else -#error Unsupported compiler / system. + #error Unsupported compiler / system. +#endif +// Include any necessary headers for the selected Atomic Implementation. +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + #include "sync.h" +#endif +#if defined(ATOMIC_USE_CPP11_ATOMIC) + #include #endif namespace srt { @@ -82,31 +120,62 @@ class atomic { sizeof(T) == 8, "Only types of size 1, 2, 4 or 8 are supported"); - atomic() : value_(static_cast(0)) {} + atomic() + : value_(static_cast(0)) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + , mutex_() +#endif + { + // No-Op + } + + explicit atomic(const T value) + : value_(value) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + , mutex_() +#endif + { + // No-Op + } - explicit atomic(const T value) : value_(value) {} + ~atomic() + { + // No-Op + } /// @brief Performs an atomic increment operation (value + 1). /// @returns The new value of the atomic object. T operator++() { -#if defined(ATOMIC_USE_GCC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + ScopedLock lg_(mutex_); + const T t = ++value_; + return t; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) return __atomic_add_fetch(&value_, 1, __ATOMIC_SEQ_CST); #elif defined(ATOMIC_USE_MSVC_INTRINSICS) return msvc::interlocked::increment(&value_); -#else +#elif defined(ATOMIC_USE_CPP11_ATOMIC) return ++value_; +#else + #error "Implement Me." #endif } /// @brief Performs an atomic decrement operation (value - 1). /// @returns The new value of the atomic object. T operator--() { -#if defined(ATOMIC_USE_GCC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + ScopedLock lg_(mutex_); + const T t = --value_; + return t; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) return __atomic_sub_fetch(&value_, 1, __ATOMIC_SEQ_CST); #elif defined(ATOMIC_USE_MSVC_INTRINSICS) return msvc::interlocked::decrement(&value_); -#else +#elif defined(ATOMIC_USE_CPP11_ATOMIC) return --value_; +#else + #error "Implement Me." #endif } @@ -119,7 +188,16 @@ class atomic { /// @param new_val The new value to write to the atomic object. /// @returns True if new_value was written to the atomic object. bool compare_exchange(const T expected_val, const T new_val) { -#if defined(ATOMIC_USE_GCC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + ScopedLock lg_(mutex_); + bool result = false; + if (expected_val == value_) + { + value_ = new_val; + result = true; + } + return result; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) T e = expected_val; return __atomic_compare_exchange_n( &value_, &e, new_val, true, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); @@ -127,9 +205,11 @@ class atomic { const T old_val = msvc::interlocked::compare_exchange(&value_, new_val, expected_val); return (old_val == expected_val); -#else +#elif defined(ATOMIC_USE_CPP11_ATOMIC) T e = expected_val; return value_.compare_exchange_weak(e, new_val); +#else + #error "Implement Me." #endif } @@ -140,12 +220,17 @@ class atomic { /// /// @param new_val The new value to write to the atomic object. void store(const T new_val) { -#if defined(ATOMIC_USE_GCC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + ScopedLock lg_(mutex_); + value_ = new_val; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) __atomic_store_n(&value_, new_val, __ATOMIC_SEQ_CST); #elif defined(ATOMIC_USE_MSVC_INTRINSICS) (void)msvc::interlocked::exchange(&value_, new_val); -#else +#elif defined(ATOMIC_USE_CPP11_ATOMIC) value_.store(new_val); +#else + #error "Implement Me." #endif } @@ -153,13 +238,19 @@ class atomic { /// @note Be careful about how this is used, since any operations on the /// returned value are inherently non-atomic. T load() const { -#if defined(ATOMIC_USE_GCC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + ScopedLock lg_(mutex_); + const T t = value_; + return t; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) return __atomic_load_n(&value_, __ATOMIC_SEQ_CST); #elif defined(ATOMIC_USE_MSVC_INTRINSICS) // TODO(m): Is there a better solution for MSVC? return value_; -#else +#elif defined(ATOMIC_USE_CPP11_ATOMIC) return value_; +#else + #error "Implement Me." #endif } @@ -171,12 +262,19 @@ class atomic { /// @param new_val The new value to write to the atomic object. /// @returns the old value. T exchange(const T new_val) { -#if defined(ATOMIC_USE_GCC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + ScopedLock lg_(mutex_); + const T t = value_; + value_ = new_val; + return t; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) return __atomic_exchange_n(&value_, new_val, __ATOMIC_SEQ_CST); #elif defined(ATOMIC_USE_MSVC_INTRINSICS) return msvc::interlocked::exchange(&value_, new_val); -#else +#elif defined(ATOMIC_USE_CPP11_ATOMIC) return value_.exchange(new_val); +#else + #error "Implement Me." #endif } @@ -190,10 +288,17 @@ class atomic { } private: -#if defined(ATOMIC_USE_GCC_INTRINSICS) || defined(ATOMIC_USE_MSVC_INTRINSICS) +#if defined(ATOMIC_USE_SRT_SYNC_MUTEX) && (ATOMIC_USE_SRT_SYNC_MUTEX == 1) + T value_; + mutable Mutex mutex_; +#elif defined(ATOMIC_USE_GCC_INTRINSICS) volatile T value_; -#else +#elif defined(ATOMIC_USE_MSVC_INTRINSICS) + volatile T value_; +#elif defined(ATOMIC_USE_CPP11_ATOMIC) std::atomic value_; +#else + #error "Implement Me. (value_ type)" #endif ATOMIC_DISALLOW_COPY(atomic) diff --git a/srtcore/atomic_clock.h b/srtcore/atomic_clock.h new file mode 100644 index 000000000..e01012313 --- /dev/null +++ b/srtcore/atomic_clock.h @@ -0,0 +1,91 @@ +/* + * SRT - Secure, Reliable, Transport + * Copyright (c) 2021 Haivision Systems Inc. + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + * + */ + +#ifndef INC_SRT_SYNC_ATOMIC_CLOCK_H +#define INC_SRT_SYNC_ATOMIC_CLOCK_H + +#include "sync.h" +#include "atomic.h" + +namespace srt +{ +namespace sync +{ + +template +class AtomicDuration +{ + atomic dur; + typedef typename Clock::duration duration_type; + typedef typename Clock::time_point time_point_type; +public: + + AtomicDuration() ATR_NOEXCEPT : dur(0) {} + + duration_type load() + { + int64_t val = dur.load(); + return duration_type(val); + } + + void store(const duration_type& d) + { + dur.store(d.count()); + } + + AtomicDuration& operator=(const duration_type& s) + { + dur = s.count(); + return *this; + } + + operator duration_type() const + { + return duration_type(dur); + } +}; + +template +class AtomicClock +{ + atomic dur; + typedef typename Clock::duration duration_type; + typedef typename Clock::time_point time_point_type; +public: + + AtomicClock() ATR_NOEXCEPT : dur(0) {} + + time_point_type load() const + { + int64_t val = dur.load(); + return time_point_type(duration_type(val)); + } + + void store(const time_point_type& d) + { + dur.store(uint64_t(d.time_since_epoch().count())); + } + + AtomicClock& operator=(const time_point_type& s) + { + dur = s.time_since_epoch().count(); + return *this; + } + + operator time_point_type() const + { + return time_point_type(duration_type(dur.load())); + } +}; + +} // namespace sync +} // namespace srt + +#endif // INC_SRT_SYNC_ATOMIC_CLOCK_H diff --git a/srtcore/sync.h b/srtcore/sync.h index d78aed980..00633da06 100644 --- a/srtcore/sync.h +++ b/srtcore/sync.h @@ -23,7 +23,6 @@ #define SRT_SYNC_CLOCK_STR "STDCXX_STEADY" #else #include -#include "atomic.h" // Defile clock type to use #ifdef IA32 @@ -233,72 +232,11 @@ inline Duration operator*(const int& lhs, const Duration -class AtomicDuration -{ - atomic dur; - typedef typename Clock::duration duration_type; - typedef typename Clock::time_point time_point_type; -public: - - AtomicDuration() ATR_NOEXCEPT : dur(0) {} - - duration_type load() - { - int64_t val = dur.load(); - return duration_type(val); - } - - void store(const duration_type& d) - { - dur.store(d.count()); - } - - AtomicDuration& operator=(const duration_type& s) - { - dur = s.count(); - return *this; - } - - operator duration_type() const - { - return duration_type(dur); - } -}; - -template -class AtomicClock -{ - atomic dur; - typedef typename Clock::duration duration_type; - typedef typename Clock::time_point time_point_type; -public: - - AtomicClock() ATR_NOEXCEPT : dur(0) {} - - time_point_type load() const - { - int64_t val = dur.load(); - return time_point_type(duration_type(val)); - } - - void store(const time_point_type& d) - { - dur.store(uint64_t(d.time_since_epoch().count())); - } - - AtomicClock& operator=(const time_point_type& s) - { - dur = s.time_since_epoch().count(); - return *this; - } - - operator time_point_type() const - { - return time_point_type(duration_type(dur.load())); - } -}; - +// NOTE: Moved the following class definitons to "atomic_clock.h" +// template +// class AtomicDuration; +// template +// class AtomicClock; /////////////////////////////////////////////////////////////////////////////// // @@ -939,4 +877,6 @@ int genRandomInt(int minVal, int maxVal); } // namespace sync } // namespace srt +#include "atomic_clock.h" + #endif // INC_SRT_SYNC_H