On 2021-04-22 02:23, Jonathan Wakely wrote:
On 21/04/21 18:29 -0700, Thomas Rodgers wrote:
From: Thomas Rodgers <rodg...@twrodgers.com>
NOTE - This patch also needs to be backported to gcc-11 in order for
semaphore release() to work correctly on non-futex platforms.
Tested sparc-sun-solaris2.11
For types that track whether or not there extant waiters (e.g.
semaphore) internally, the __atomic_notify_address_bare() call was
introduced to avoid the overhead of loading the atomic count of
waiters. For platforms that don't have Futex, however, there was
still a check for waiters, and seeing that there are none (because
in the bare case, the count is not incremented), the notification
is dropped. This commit addresses that case.
libstdc++-v3/ChangeLog:
* include/bits/atomic_wait.h: Always notify waiters in the
in the case of 'bare' address notification.
Repeated text: "in the in the"
---
libstdc++-v3/include/bits/atomic_wait.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libstdc++-v3/include/bits/atomic_wait.h
b/libstdc++-v3/include/bits/atomic_wait.h
index 0ac5575190c..984ed70f16c 100644
--- a/libstdc++-v3/include/bits/atomic_wait.h
+++ b/libstdc++-v3/include/bits/atomic_wait.h
@@ -226,9 +226,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
void
- _M_notify(const __platform_wait_t* __addr, bool __all) noexcept
+ _M_notify(const __platform_wait_t* __addr, bool __all, bool
__bare) noexcept
{
- if (!_M_waiting())
+ if (!(__bare || _M_waiting()))
Maybe it's just me, but I would find (!__base && !__waiting) to be a
clearer expression of the logic here.
i.e. "return if the wait is not bare and there are no waiters"
rather than "return if the wait is not either bare or has waiters".
The latter makes me take a second to grok.
As discussed on IRC, I went back and forth on this a couple of times and
neither option seemed particularly great to me. I started down the path
of propagating the std::true_type/std::false_type bits that the RAII
wrapper type knows about and making this a compile time choice, but felt
the change was too big to make this late in the release cycle. I will
revisit this for stage1 (though ideally all of this will be moved into
the .so for GCC12 and partially rewritten as part of that process
anyway).
The patch is OK either way though, with the ChangeLog typo fix.
return;
#ifdef _GLIBCXX_HAVE_PLATFORM_WAIT
@@ -304,11 +304,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
void
- _M_notify(bool __all)
+ _M_notify(bool __all, bool __bare = false)
{
if (_M_addr == &_M_w._M_ver)
__atomic_fetch_add(_M_addr, 1, __ATOMIC_ACQ_REL);
- _M_w._M_notify(_M_addr, __all);
+ _M_w._M_notify(_M_addr, __all, __bare);
}
template<typename _Up, typename _ValFn,
@@ -452,7 +452,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__atomic_notify_address(const _Tp* __addr, bool __all) noexcept
{
__detail::__bare_wait __w(__addr);
- __w._M_notify(__all);
+ __w._M_notify(__all, true);
}
// This call is to be used by atomic types which track contention
externally
@@ -464,7 +464,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__detail::__platform_notify(__addr, __all);
#else
__detail::__bare_wait __w(__addr);
- __w._M_notify(__all);
+ __w._M_notify(__all, true);
#endif
}
_GLIBCXX_END_NAMESPACE_VERSION
-- 2.30.2
Tested sparc-sun-solaris2.11.
Committed to master, backported to release/gcc-11.
