On 20/04/21 15:25 +0100, Jonathan Wakely wrote: >On 20/04/21 12:41 +0100, Jonathan Wakely wrote: >>On 20/04/21 12:04 +0100, Jonathan Wakely wrote: >>>On 19/04/21 12:23 -0700, Thomas Rodgers wrote: >>>>From: Thomas Rodgers >>>> >>>>This patch address jwakely's feedback from 2021-04-15. >>>> >>>>This is a substantial rewrite of the atomic wait/notify (and timed wait >>>>counterparts) implementation. >>>> >>>>The previous __platform_wait looped on EINTR however this behavior is >>>>not required by the standard. A new _GLIBCXX_HAVE_PLATFORM_WAIT macro >>>>now controls whether wait/notify are implemented using a platform >>>>specific primitive or with a platform agnostic mutex/condvar. This >>>>patch only supplies a definition for linux futexes. A future update >>>>could add support __ulock_wait/wake on Darwin, for instance. >>>> >>>>The members of __waiters were lifted to a new base class. The members >>>>are now arranged such that overall sizeof(__waiters_base) fits in two >>>>cache lines (on platforms with at least 64 byte cache lines). The >>>>definition will also use destructive_interference_size for this if it >>>>is available. >>>> >>>>The __waiters type is now specific to untimed waits. Timed waits have a >>>>corresponding __timed_waiters type. Much of the code has been moved from >>>>the previous __atomic_wait() free function to the __waiter_base template >>>>and a __waiter derived type is provided to implement the un-timed wait >>>>operations. A similar change has been made to the timed wait >>>>implementation. >>>> >>>>The __atomic_spin code has been extended to take a spin policy which is >>>>invoked after the initial busy wait loop. The default policy is to >>>>return from the spin. The timed wait code adds a timed backoff spinning >>>>policy. The code from which implements this_thread::sleep_for, >>>>sleep_until has been moved to a new header >>> >>>The commit msg wasn't updated for the latest round of changes >>>(this_thread_sleep, __waiters_pool_base etc). >>> >>>>which allows the thread sleep code to be consumed without pulling in the >>>>whole of . >>>> >>>>The entry points into the wait/notify code have been restructured to >>>>support either - >>>>* Testing the current value of the atomic stored at the given address >>>> and waiting on a notification. >>>>* Applying a predicate to determine if the wait was satisfied. >>>>The entry points were renamed to make it clear that the wait and wake >>>>operations operate on addresses. The first variant takes the expected >>>>value and a function which returns the current value that should be used >>>>in comparison operations, these operations are named with a _v suffix >>>>(e.g. 'value'). All atomic<_Tp> wait/notify operations use the first >>>>variant. Barriers, latches and semaphores use the predicate variant. >>>> >>>>This change also centralizes what it means to compare values for the >>>>purposes of atomic::wait rather than scattering through individual >>>>predicates. >>>> >>>>This change also centralizes the repetitive code which adjusts for >>>>different user supplied clocks (this should be moved elsewhere >>>>and all such adjustments should use a common implementation). >>>> >>>>This change also removes the hashing of the pointer and uses >>>>the pointer value directly for indexing into the waiters table. >>>> >>>>libstdc++-v3/ChangeLog: >>>> * include/Makefile.am: Add new header. >>> >>>The name needs updating to correspond to the latest version of the >>>patch. >>> >>>> * include/Makefile.in: Regenerate. >>>> * include/bits/atomic_base.h: Adjust all calls >>>> to __atomic_wait/__atomic_notify for new call signatures. >>>> * include/bits/atomic_wait.h: Extensive rewrite. >>>> * include/bits/atomic_timed_wait.h: Likewise. >>>> * include/bits/semaphore_base.h: Adjust all calls >>>> to __atomic_wait/__atomic_notify for new call signatures. >>>> * include/bits/this_thread_sleep.h: New file. >>>> * include/std/atomic: Likewise. >>>> * include/std/barrier: Likewise. >>>> * include/std/latch: Likewise. >>> >>>include/std/thread is missing from the changelog entry. You can use >>>the 'git gcc-verify' alias to check your commit log will be accepted >>>by the server-side hook: >>> >>>'gcc-verify' is aliased to '!f() { "`git rev-parse --show-toplevel`/contrib/gcc-changelog/git_check_commit.py" $@; } ; f' >>> >>> >>>> * testsuite/29_atomics/atomic/wait_notify/bool.cc: Simplify >>>> test. >>>> * testsuite/29_atomics/atomic/wait_notify/generic.cc: Likewise. >>>> * testsuite/29_atomics/atomic/wait_notify/pointers.cc: Likewise. >>>> * testsuite/29_atomics/atomic_flag/wait_notify.cc: Likewise. >>>> * testsuite/29_atomics/atomic_float/wait_notify.cc: Likewise. >>>> * testsuite/29_atomics/atomic_integral/wait_notify.cc: Likewise. >>>> * testsuite/29_atomics/atomic_ref/wait_notify.cc: Likewise. >>> >>>>- struct __timed_waiters : __waiters >>>>+ struct __timed_waiters : __waiter_pool_base >>> >>>Should this be __timed_waiter_pool for consistency with >>>__waiter_pool_base and __waiter_pool? >>> >>> >>>>- inline void >>>>- __thread_relax() noexcept >>>>- { >>>>-#if defined __i386__ || defined __x86_64__ >>>>- __builtin_ia32_pause(); >>>>-#elif defined _GLIBCXX_USE_SCHED_YIELD >>>>- __gthread_yield(); >>>>-#endif >>>>- } >>>>+ template >>>>+ struct __waiter_base >>>>+ { >>>>+ using __waiter_type = _Tp; >>>> >>>>- inline void >>>>- __thread_yield() noexcept >>>>- { >>>>-#if defined _GLIBCXX_USE_SCHED_YIELD >>>>- __gthread_yield(); >>>>-#endif >>>>- } >>> >>>This chunk of the patch doesn't apply, because it's based on an old >>>version of trunk (before r11-7248). >> >>I managed to bodge the patch so it applies, see attached. > >The attached patch is what I've pushed to trunk and gcc-11, which >addresses all my comments from today. And this disables some tests that are failing consistently (either on all targets, or solaris). Also pushed to trunk and gcc-11. I've also just seen this one on solaris: WARNING: program timed out. FAIL: 30_threads/barrier/arrive_and_wait.cc execution test These need to be analysed and the tests re-enabled.