From 9d8de7f12c8cd4f96fd907da1344a9f209bc4eba Mon Sep 17 00:00:00 2001 From: Ben Deane Date: Wed, 16 Oct 2024 12:19:49 -0600 Subject: [PATCH] :sparkles: Add `atomic` Problem: - Using `std::atomic` does not necessarily produce good codegen for microcontroller platforms. Solution: - Add `stdx::atomic` which uses the customizable atomic interface from the baremetal concurrency library. --- CMakeLists.txt | 3 +- docs/atomic.adoc | 49 ++++++++++ docs/index.adoc | 1 + docs/intro.adoc | 1 + include/stdx/atomic.hpp | 122 ++++++++++++++++++++++++ test/CMakeLists.txt | 6 ++ test/atomic.cpp | 161 ++++++++++++++++++++++++++++++++ test/atomic_bitset.cpp | 2 +- test/atomic_bitset_override.cpp | 4 +- test/atomic_override.cpp | 26 ++++++ test/detail/atomic_cfg.hpp | 9 +- test/fail/CMakeLists.txt | 1 + test/fail/atomic_bool_dec.cpp | 8 ++ 13 files changed, 387 insertions(+), 6 deletions(-) create mode 100644 docs/atomic.adoc create mode 100644 include/stdx/atomic.hpp create mode 100644 test/atomic.cpp create mode 100644 test/atomic_override.cpp create mode 100644 test/fail/atomic_bool_dec.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 3a9108b..989a931 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,7 +16,7 @@ endif() add_versioned_package("gh:boostorg/mp11#boost-1.83.0") fmt_recipe(10.2.1) -add_versioned_package("gh:intel/cpp-baremetal-concurrency#27de8e1") +add_versioned_package("gh:intel/cpp-baremetal-concurrency#06e5901") if(NOT DEFINED CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 20) @@ -39,6 +39,7 @@ target_sources( include FILES include/stdx/algorithm.hpp + include/stdx/atomic.hpp include/stdx/atomic_bitset.hpp include/stdx/bit.hpp include/stdx/bitset.hpp diff --git a/docs/atomic.adoc b/docs/atomic.adoc new file mode 100644 index 0000000..b9b1de1 --- /dev/null +++ b/docs/atomic.adoc @@ -0,0 +1,49 @@ + +== `atomic.hpp` + +https://github.com/intel/cpp-std-extensions/blob/main/include/stdx/atomic.hpp[`atomic.hpp`] +provides an implementation of +https://en.cppreference.com/w/cpp/atomic/atomic[`std::atomic`] with a few +differences. + +`stdx::atomic` does not implement: + + * `is_lock_free` or `is_always_lock_free` + * `compare_exchange_{weak,strong}` + * `wait` + * `notify_{one,all}` + * `fetch_{max,min}` + +However, `stdx::atomic` allows customization of the atomic implementation for +best codegen. `stdx::atomic` is implemented using the atomic API exposed by +Intel's https://github.com/intel/cpp-baremetal-concurrency[baremetal concurrency +library]. + +For example, it is possible that a particular platform requires atomic accesses +to be 32-bit aligned. To achieve that for `stdx::atomic`, we could provide a +configuration header specializing `::atomic::alignment_of`: + +[source,cpp] +---- +// this header: atomic_cfg.hpp +#include + +template <> +constexpr inline auto ::atomic::alignment_of = alignof(std::uint32_t); +---- + +To apply this configuration, when compiling, pass `-DATOMIC_CFG="/atomic_cfg.hpp"`. +The result would be that `stdx::atomic` has 32-bit alignment: + +[source,cpp] +---- +static_assert(alignof(stdx::atomic) == alignof(std::uint32_t)); +---- + +Using the https://github.com/intel/cpp-baremetal-concurrency[baremetal +concurrency library] it is possible to override the handling of atomic access +(`load`, `store`, `exchange`, `fetch_`) to ensure the best codegen on a +particular platform. As well as alignment concerns, for instance it may be the +case on a single-core microcontroller that it is cheaper to disable and +re-enable interrupts around a read/write than incurring a lock-free atomic +access. diff --git a/docs/index.adoc b/docs/index.adoc index 8809226..b2689fe 100644 --- a/docs/index.adoc +++ b/docs/index.adoc @@ -7,6 +7,7 @@ :toc: left include::intro.adoc[] +include::atomic.adoc[] include::atomic_bitset.adoc[] include::algorithm.adoc[] include::bit.adoc[] diff --git a/docs/intro.adoc b/docs/intro.adoc index ea33c3e..64495c2 100644 --- a/docs/intro.adoc +++ b/docs/intro.adoc @@ -35,6 +35,7 @@ into headers whose names match the standard. The following headers are available: * https://github.com/intel/cpp-std-extensions/blob/main/include/stdx/algorithm.hpp[`algorithm.hpp`] +* https://github.com/intel/cpp-std-extensions/blob/main/include/stdx/atomic.hpp[`atomic.hpp`] * https://github.com/intel/cpp-std-extensions/blob/main/include/stdx/atomic_bitset.hpp[`atomic_bitset.hpp`] * https://github.com/intel/cpp-std-extensions/blob/main/include/stdx/bit.hpp[`bit.hpp`] * https://github.com/intel/cpp-std-extensions/blob/main/include/stdx/bitset.hpp[`bitset.hpp`] diff --git a/include/stdx/atomic.hpp b/include/stdx/atomic.hpp new file mode 100644 index 0000000..dc81f64 --- /dev/null +++ b/include/stdx/atomic.hpp @@ -0,0 +1,122 @@ +#pragma once + +#include + +#include +#include + +#if __cplusplus >= 202002L +#define CPP20(...) __VA_ARGS__ +#else +#define CPP20(...) +#endif + +namespace stdx { +inline namespace v1 { +template class atomic { + static_assert(std::is_trivially_copyable_v and + std::is_copy_constructible_v and + std::is_move_constructible_v and + std::is_copy_assignable_v and + std::is_move_assignable_v, + "Atomic values must be trivially copyable, copy " + "constructible and copy assignable"); + + using elem_t = ::atomic::atomic_type_t; + constexpr static auto alignment = ::atomic::alignment_of; + + static_assert(std::is_convertible_v, + "::atomic::atomic_type_t specialization result must be " + "convertible to T"); + static_assert(std::is_convertible_v, + "::atomic::atomic_type_t specialization result must be " + "convertible from T"); + + alignas(alignment) elem_t value; + + public: + using value_type = T; + + constexpr atomic() CPP20(requires std::is_default_constructible_v) + : value{} {} + constexpr atomic(T t) : value{static_cast(t)} {} + atomic(atomic const &) = delete; + auto operator=(atomic const &) -> atomic & = delete; + + [[nodiscard]] auto + load(std::memory_order mo = std::memory_order_seq_cst) const -> T { + return static_cast(::atomic::load(value, mo)); + } + + void store(T t, std::memory_order mo = std::memory_order_seq_cst) { + ::atomic::store(value, static_cast(t), mo); + } + + [[nodiscard]] operator T() const { return load(); } + auto operator=(T t) -> T { + store(t); + return t; + } + + [[nodiscard]] auto + exchange(T t, std::memory_order mo = std::memory_order_seq_cst) -> T { + return ::atomic::exchange(value, static_cast(t), mo); + } + + auto fetch_add(T t, std::memory_order mo = std::memory_order_seq_cst) -> T { + CPP20(static_assert( + requires { t + t; }, "T must support operator+(x, y)")); + return ::atomic::fetch_add(value, static_cast(t), mo); + } + auto fetch_sub(T t, std::memory_order mo = std::memory_order_seq_cst) -> T { + CPP20(static_assert( + requires { t - t; }, "T must support operator-(x, y)")); + return ::atomic::fetch_sub(value, static_cast(t), mo); + } + + auto operator+=(T t) -> T { return fetch_add(t) + t; } + auto operator-=(T t) -> T { return fetch_sub(t) - t; } + + auto operator++() -> T { + CPP20(static_assert( + requires(T t) { ++t; }, "T must support operator++()")); + return ::atomic::fetch_add(value, 1) + 1; + } + [[nodiscard]] auto operator++(int) -> T { + CPP20(static_assert( + requires(T t) { t++; }, "T must support operator++(int)")); + return ::atomic::fetch_add(value, 1); + } + auto operator--() -> T { + CPP20(static_assert( + requires(T t) { --t; }, "T must support operator--()")); + return ::atomic::fetch_sub(value, 1) - 1; + } + [[nodiscard]] auto operator--(int) -> T { + CPP20(static_assert( + requires(T t) { t--; }, "T must support operator--(int)")); + return ::atomic::fetch_sub(value, 1); + } + + auto fetch_and(T t, std::memory_order mo = std::memory_order_seq_cst) -> T { + CPP20(static_assert( + requires { t & t; }, "T must support operator&(x, y)")); + return ::atomic::fetch_and(value, static_cast(t), mo); + } + auto fetch_or(T t, std::memory_order mo = std::memory_order_seq_cst) -> T { + CPP20(static_assert( + requires { t | t; }, "T must support operator|(x, y)")); + return ::atomic::fetch_or(value, static_cast(t), mo); + } + auto fetch_xor(T t, std::memory_order mo = std::memory_order_seq_cst) -> T { + CPP20(static_assert( + requires { t ^ t; }, "T must support operator^(x, y)")); + return ::atomic::fetch_xor(value, static_cast(t), mo); + } + + auto operator&=(T t) -> T { return fetch_and(t) & t; } + auto operator|=(T t) -> T { return fetch_or(t) | t; } + auto operator^=(T t) -> T { return fetch_xor(t) ^ t; } +}; +} // namespace v1 +} // namespace stdx diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 53951b6..08448aa 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -21,6 +21,8 @@ add_tests( FILES algorithm always_false + atomic + atomic_override atomic_bitset atomic_bitset_override bind @@ -65,6 +67,10 @@ target_compile_definitions( atomic_bitset_override_test PRIVATE -DATOMIC_CFG="${CMAKE_CURRENT_LIST_DIR}/detail/atomic_cfg.hpp") +target_compile_definitions( + atomic_override_test + PRIVATE -DATOMIC_CFG="${CMAKE_CURRENT_LIST_DIR}/detail/atomic_cfg.hpp") + if(${CMAKE_CXX_STANDARD} GREATER_EQUAL 20) add_tests(FILES ct_format ct_string indexed_tuple tuple tuple_algorithms) endif() diff --git a/test/atomic.cpp b/test/atomic.cpp new file mode 100644 index 0000000..4e7b386 --- /dev/null +++ b/test/atomic.cpp @@ -0,0 +1,161 @@ +#include + +#include +#include + +#include +#include + +TEMPLATE_TEST_CASE("atomic size and alignment is the same as the data", + "[atomic]", bool, char, signed char, unsigned char, + short int, unsigned short int, int, unsigned int, long int, + unsigned long int) { + static_assert(sizeof(stdx::atomic) == sizeof(TestType)); + static_assert(alignof(stdx::atomic) == alignof(TestType)); +} + +TEMPLATE_TEST_CASE("atomic is default constructible when data is", "[atomic]", + bool, char, signed char, unsigned char, short int, + unsigned short int, int, unsigned int, long int, + unsigned long int) { + static_assert(std::is_default_constructible_v>); +} + +namespace { +struct non_dc { + non_dc(int) {} +}; +} // namespace + +#if __cplusplus >= 202002L +TEST_CASE("atomic is not default constructible when data is not", "[atomic]") { + static_assert(not std::is_default_constructible_v>); +} +#endif + +TEST_CASE("atomic is not copyable or movable", "[atomic]") { + static_assert(not std::is_copy_constructible_v>); + static_assert(not std::is_move_constructible_v>); + static_assert(not std::is_copy_assignable_v>); + static_assert(not std::is_move_assignable_v>); +} + +TEMPLATE_TEST_CASE("atomic supports value initialization", "[atomic]", bool, + char, signed char, unsigned char, short int, + unsigned short int, int, unsigned int, long int, + unsigned long int) { + static_assert(std::is_constructible_v, TestType>); + [[maybe_unused]] auto x = stdx::atomic{TestType{}}; +} + +TEST_CASE("load", "[atomic]") { + stdx::atomic val{17}; + CHECK(val.load() == 17); +} + +TEST_CASE("store", "[atomic]") { + stdx::atomic val{17}; + val.store(1337); + CHECK(val.load() == 1337); +} + +TEST_CASE("implicit conversion to T", "[atomic]") { + stdx::atomic val{17}; + CHECK(val == 17); +} + +TEST_CASE("assignment from T", "[atomic]") { + stdx::atomic val{17}; + val = 1337; + CHECK(val == 1337); +} + +TEST_CASE("exchange", "[atomic]") { + stdx::atomic val{17}; + CHECK(val.exchange(1337) == 17); + CHECK(val.load() == 1337); +} + +TEST_CASE("fetch_add", "[atomic]") { + stdx::atomic val{17}; + CHECK(val.fetch_add(42) == 17); + CHECK(val.load() == 59); +} + +TEST_CASE("fetch_sub", "[atomic]") { + stdx::atomic val{59}; + CHECK(val.fetch_sub(42) == 59); + CHECK(val.load() == 17); +} + +TEST_CASE("operator +=", "[atomic]") { + stdx::atomic val{17}; + CHECK((val += 42) == 59); + CHECK(val.load() == 59); +} + +TEST_CASE("operator -=", "[atomic]") { + stdx::atomic val{59}; + CHECK((val -= 42) == 17); + CHECK(val.load() == 17); +} + +TEST_CASE("pre-increment", "[atomic]") { + stdx::atomic val{17}; + CHECK(++val == 18); + CHECK(val.load() == 18); +} + +TEST_CASE("post-increment", "[atomic]") { + stdx::atomic val{17}; + CHECK(val++ == 17); + CHECK(val.load() == 18); +} + +TEST_CASE("pre-decrement", "[atomic]") { + stdx::atomic val{17}; + CHECK(--val == 16); + CHECK(val.load() == 16); +} + +TEST_CASE("post-decrement", "[atomic]") { + stdx::atomic val{17}; + CHECK(val-- == 17); + CHECK(val.load() == 16); +} + +TEST_CASE("fetch_and", "[atomic]") { + stdx::atomic val{0b101}; + CHECK(val.fetch_and(0b100) == 0b101); + CHECK(val.load() == 0b100); +} + +TEST_CASE("fetch_or", "[atomic]") { + stdx::atomic val{0b1}; + CHECK(val.fetch_or(0b100) == 0b1); + CHECK(val.load() == 0b101); +} + +TEST_CASE("fetch_xor", "[atomic]") { + stdx::atomic val{0b101}; + CHECK(val.fetch_xor(0b1) == 0b101); + CHECK(val.load() == 0b100); +} + +TEST_CASE("operator &=", "[atomic]") { + stdx::atomic val{0b101}; + CHECK((val &= 0b100) == 0b100); + CHECK(val.load() == 0b100); +} + +TEST_CASE("operator |=", "[atomic]") { + stdx::atomic val{0b1}; + CHECK((val |= 0b100) == 0b101); + CHECK(val.load() == 0b101); +} + +TEST_CASE("operator ^=", "[atomic]") { + stdx::atomic val{0b101}; + CHECK((val ^= 0b1) == 0b100); + CHECK(val.load() == 0b100); +} diff --git a/test/atomic_bitset.cpp b/test/atomic_bitset.cpp index 4dfad11..0a42ff6 100644 --- a/test/atomic_bitset.cpp +++ b/test/atomic_bitset.cpp @@ -162,7 +162,7 @@ TEST_CASE("to_natural returns smallest_uint", "[atomic_bitset]") { auto bs = stdx::atomic_bitset<4>{stdx::all_bits}; auto value = bs.to_natural(); CHECK(value == 0b1111); - static_assert(std::same_as); + static_assert(std::is_same_v); } TEMPLATE_TEST_CASE("construct with a string_view", "[atomic_bitset]", diff --git a/test/atomic_bitset_override.cpp b/test/atomic_bitset_override.cpp index 7f8c901..0c17596 100644 --- a/test/atomic_bitset_override.cpp +++ b/test/atomic_bitset_override.cpp @@ -2,8 +2,8 @@ #include -#include #include +#include TEST_CASE("atomic_bitset works with overridden type", "[atomic_bitset_override]") { @@ -16,5 +16,5 @@ TEST_CASE("to_natural returns smallest_uint", "[atomic_bitset_override]") { auto bs = stdx::atomic_bitset<4>{stdx::all_bits}; auto value = bs.to_natural(); CHECK(value == 0b1111); - static_assert(std::same_as); + static_assert(std::is_same_v); } diff --git a/test/atomic_override.cpp b/test/atomic_override.cpp new file mode 100644 index 0000000..5774661 --- /dev/null +++ b/test/atomic_override.cpp @@ -0,0 +1,26 @@ +#include + +#include + +#include +#include + +TEST_CASE("atomic with overridden type is correctly sized/aligned", + "[atomic_override]") { + auto bs = stdx::atomic{}; + static_assert(sizeof(decltype(bs)) == sizeof(std::uint32_t)); + static_assert(alignof(decltype(bs)) == alignof(std::uint32_t)); +} + +TEST_CASE("atomic with overridden type presents interface of original type", + "[atomic_override]") { + auto bs = stdx::atomic{}; + static_assert(std::is_same_v); +} + +TEST_CASE("atomic works with overridden type", "[atomic_override]") { + auto bs = stdx::atomic{}; + CHECK(!bs); + CHECK(!bs.exchange(true)); + CHECK(bs); +} diff --git a/test/detail/atomic_cfg.hpp b/test/detail/atomic_cfg.hpp index b27f53c..3ac9212 100644 --- a/test/detail/atomic_cfg.hpp +++ b/test/detail/atomic_cfg.hpp @@ -3,5 +3,10 @@ #include #include -template <> -struct atomic::atomic_type : std::type_identity {}; +template <> struct atomic::atomic_type { + using type = std::uint32_t; +}; + +template <> struct atomic::atomic_type { + using type = std::uint32_t; +}; diff --git a/test/fail/CMakeLists.txt b/test/fail/CMakeLists.txt index 4b9e93c..e5c7e53 100644 --- a/test/fail/CMakeLists.txt +++ b/test/fail/CMakeLists.txt @@ -35,6 +35,7 @@ if(${CMAKE_CXX_STANDARD} GREATER_EQUAL 20) endif() add_fail_tests( + atomic_bool_dec dynamic_span_no_ct_capacity dynamic_container_no_ct_capacity tuple_index_out_of_bounds diff --git a/test/fail/atomic_bool_dec.cpp b/test/fail/atomic_bool_dec.cpp new file mode 100644 index 0000000..a9748b7 --- /dev/null +++ b/test/fail/atomic_bool_dec.cpp @@ -0,0 +1,8 @@ +#include + +// EXPECT: T must support operator-- + +auto main() -> int { + auto x = stdx::atomic{true}; + --x; +}