aboutsummaryrefslogtreecommitdiff
path: root/libcxx/include/semaphore
diff options
context:
space:
mode:
authorJan Kokemüller <jan.kokemueller@gmail.com>2024-02-19 15:28:51 +0100
committerGitHub <noreply@github.com>2024-02-19 14:28:51 +0000
commit95ebf2be0e6600465a4d0f4e7d81c7ded0559fba (patch)
tree22e2d5df0118ba9dbdd931ebad1a39c1cf80fe2c /libcxx/include/semaphore
parent3f0404aae7ed2f7138526e1bcd100a60dfe08227 (diff)
downloadllvm-95ebf2be0e6600465a4d0f4e7d81c7ded0559fba.zip
llvm-95ebf2be0e6600465a4d0f4e7d81c7ded0559fba.tar.gz
llvm-95ebf2be0e6600465a4d0f4e7d81c7ded0559fba.tar.bz2
[libc++] Refactor the predicate taking variant of `__cxx_atomic_wait` (#80596)
This is a follow-up PR to <https://github.com/llvm/llvm-project/pull/79265>. It aims to be a gentle refactoring of the `__cxx_atomic_wait` function that takes a predicate. The key idea here is that this function's signature is changed to look like this (`std::function` used just for clarity): ```c++ __cxx_atomic_wait_fn(Atp*, std::function<bool(Tp &)> poll, memory_order __order); ``` ...where `Tp` is the corresponding `value_type` to the atomic variable type `Atp`. The function's semantics are similar to `atomic`s `.wait()`, but instead of having a hardcoded predicate (is the loaded value unequal to `old`?) the predicate is specified explicitly. The `poll` function may change its argument, and it is very important that if it returns `false`, it leaves its current understanding of the atomic's value in the argument. Internally, `__cxx_atomic_wait_fn` dispatches to two waiting mechanisms, depending on the type of the atomic variable: 1. If the atomic variable can be waited on directly (for example, Linux's futex mechanism only supports waiting on 32 bit long variables), the value of the atomic variable (which `poll` made its decision on) is then given to the underlying system wait function (e.g. futex). 2. If the atomic variable can not be waited on directly, there is a global pool of atomics that are used for this task. The ["eventcount" pattern](<https://gist.github.com/mratsim/04a29bdd98d6295acda4d0677c4d0041>) is employed to make this possible. The eventcount pattern needs a "monitor" variable which is read before the condition is checked another time. libcxx has the `__libcpp_atomic_monitor` function for this. However, this function only has to be called in case "2", i.e. when the eventcount is actually used. In case "1", the futex is used directly, so the monitor must be the value of the atomic variable that the `poll` function made its decision on to continue blocking. Previously, `__libcpp_atomic_monitor` was _also_ used in case "1". This was the source of the ABA style bug that PR#79265 fixed. However, the solution in PR#79265 has some disadvantages: - It exposes internals such as `cxx_contention_t` or the fact that `__libcpp_thread_poll_with_backoff` needs two functions to higher level constructs such as `semaphore`. - It doesn't prevent consumers calling `__cxx_atomic_wait` in an error prone way, i.e. by providing to it a predicate that doesn't take an argument. This makes ABA style issues more likely to appear. Now, `__cxx_atomic_wait_fn` takes just _one_ function, which is then transformed into the `poll` and `backoff` callables needed by `__libcpp_thread_poll_with_backoff`. Aside from the `__cxx_atomic_wait` changes, the only other change is the weakening of the initial atomic load of `semaphore`'s `try_acquire` into `memory_order_relaxed` and the CAS inside the loop is changed from `strong` to `weak`. Both weakenings should be fine, since the CAS is called in a loop, and the "acquire" semantics of `try_acquire` come from the CAS, not from the initial load.
Diffstat (limited to 'libcxx/include/semaphore')
-rw-r--r--libcxx/include/semaphore18
1 files changed, 4 insertions, 14 deletions
diff --git a/libcxx/include/semaphore b/libcxx/include/semaphore
index 5235d72..448b5fb 100644
--- a/libcxx/include/semaphore
+++ b/libcxx/include/semaphore
@@ -54,7 +54,6 @@ using binary_semaphore = counting_semaphore<1>;
#include <__assert> // all public C++ headers provide the assertion handler
#include <__atomic/atomic_base.h>
#include <__atomic/atomic_sync.h>
-#include <__atomic/contention_t.h>
#include <__atomic/memory_order.h>
#include <__availability>
#include <__chrono/time_point.h>
@@ -100,17 +99,8 @@ public:
}
}
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI void acquire() {
- auto const __poll_fn = [this]() -> bool {
- auto __old = __a_.load(memory_order_relaxed);
- return (__old != 0) && __a_.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed);
- };
- auto const __backoff_test = [this](__cxx_contention_t& __monitor) -> bool {
- ptrdiff_t __old = __monitor;
- bool __r = __try_acquire_impl(__old);
- __monitor = static_cast<__cxx_contention_t>(__old);
- return __r;
- };
- __cxx_atomic_wait(&__a_.__a_, __poll_fn, __backoff_test);
+ __cxx_atomic_wait_unless(
+ &__a_.__a_, [this](ptrdiff_t& __old) { return __try_acquire_impl(__old); }, memory_order_relaxed);
}
template <class _Rep, class _Period>
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool
@@ -121,7 +111,7 @@ public:
return std::__libcpp_thread_poll_with_backoff(__poll_fn, __libcpp_timed_backoff_policy(), __rel_time);
}
_LIBCPP_AVAILABILITY_SYNC _LIBCPP_HIDE_FROM_ABI bool try_acquire() {
- auto __old = __a_.load(memory_order_acquire);
+ auto __old = __a_.load(memory_order_relaxed);
return __try_acquire_impl(__old);
}
@@ -130,7 +120,7 @@ private:
while (true) {
if (__old == 0)
return false;
- if (__a_.compare_exchange_strong(__old, __old - 1, memory_order_acquire, memory_order_relaxed))
+ if (__a_.compare_exchange_weak(__old, __old - 1, memory_order_acquire, memory_order_relaxed))
return true;
}
}