On 05/10/20 15:54 -0700, Thomas Rodgers wrote:
From: Thomas Rodgers <trodg...@redhat.com>

This *should* be the correct patch this time.

There are still some review comments not addressed. I'll keep all my
comments on this version in a single reply, so they aren't lost among
multiple replies.

I think the significant issues like the handling of spurious wakeups
and the missing "no effects" case in _M_try_acquire() are resolved,
thanks.

diff --git a/libstdc++-v3/include/bits/atomic_timed_wait.h 
b/libstdc++-v3/include/bits/atomic_timed_wait.h
new file mode 100644
index 00000000000..cd5b6aabc44
--- /dev/null
+++ b/libstdc++-v3/include/bits/atomic_timed_wait.h

[...]

+    struct __timed_waiters : __waiters
+    {
+      template<typename _Clock, typename _Duration>
+       __atomic_wait_status
+       _M_do_wait_until(__platform_wait_t __version,
+                        const chrono::time_point<_Clock, _Duration>& __atime)
+       {
+#ifdef _GLIBCXX_HAVE_LINUX_FUTEX
+         return __platform_wait_until(&_M_ver, __version, __atime);

Qualify to prevent ADL in associated namespaces of the _Clock and
_Duration types.

+  template<typename _Tp, typename _Pred,
+          typename _Clock, typename _Duration>
+    bool
+    __atomic_wait_until(const _Tp* __addr, _Tp __old, _Pred __pred,
+                       const chrono::time_point<_Clock, _Duration>&
+                           __atime) noexcept
+    {
+      using namespace __detail;
+
+      if (std::__atomic_spin(__pred))
+       return true;
+
+      auto& __w = __timed_waiters::_S_timed_for((void*)__addr);
+      auto __version = __w._M_enter_wait();
+      do
+       {
+         __atomic_wait_status __res;
+         if constexpr (__platform_wait_uses_type<_Tp>)
+           {
+             __res = __platform_wait_until((__platform_wait_t*)(void*) __addr,

Qualify to prevent ADL.


diff --git a/libstdc++-v3/include/bits/atomic_wait.h 
b/libstdc++-v3/include/bits/atomic_wait.h
new file mode 100644
index 00000000000..30f682e828a
--- /dev/null
+++ b/libstdc++-v3/include/bits/atomic_wait.h


+  template<typename _Tp, typename _Pred>
+    void
+    __atomic_wait(const _Tp* __addr, _Tp __old, _Pred __pred) noexcept
+    {
+      using namespace __detail;
+      if (__atomic_spin(__pred))

Qualify to prevent ADL.

diff --git a/libstdc++-v3/include/bits/semaphore_base.h 
b/libstdc++-v3/include/bits/semaphore_base.h
new file mode 100644
index 00000000000..a32a6b12a1b
--- /dev/null
+++ b/libstdc++-v3/include/bits/semaphore_base.h
@@ -0,0 +1,298 @@
+// -*- C++ -*- header.
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+// <http://www.gnu.org/licenses/>.
+
+/** @file bits/semaphore_base.h
+ *  This is an internal header file, included by other library headers.
+ *  Do not attempt to use it directly. @headername{semaphore}
+ */
+
+#ifndef _GLIBCXX_SEMAPHORE_BASE_H
+#define _GLIBCXX_SEMAPHORE_BASE_H 1
+
+#pragma GCC system_header
+
+#include <bits/c++config.h>
+#include <bits/atomic_base.h>
+#include <bits/atomic_timed_wait.h>
+
+#include <ext/numeric_traits.h>
+
+#if __has_include(<semaphore.h>)
+#define _GLIBCXX_HAVE_POSIX_SEMAPHORE 1
+#include <semaphore.h>
+#endif
+
+#include <chrono>
+#include <type_traits>
+
+#include <iostream>

This shouldn't be here.

+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
+  struct __platform_semaphore
+  {
+    using __clock_t = chrono::system_clock;
+    static constexpr ptrdiff_t _S_max = SEM_VALUE_MAX;
+
+    explicit __platform_semaphore(ptrdiff_t __count) noexcept
+    {
+      sem_init(&_M_semaphore, 0, __count);
+    }
+
+    __platform_semaphore(const __platform_semaphore&) = delete;
+    __platform_semaphore& operator=(const __platform_semaphore&) = delete;
+
+    ~__platform_semaphore()
+    { sem_destroy(&_M_semaphore); }
+
+    _GLIBCXX_ALWAYS_INLINE void
+    _M_acquire() noexcept
+    {
+      for (;;)
+       {
+         auto __err = sem_wait(&_M_semaphore);
+         if (__err && (errno == EINTR))
+           continue;
+         else if (__err)
+           std::terminate();
+         else
+           break;
+       }
+    }
+
+    _GLIBCXX_ALWAYS_INLINE void
+    _M_release(std::ptrdiff_t __update) noexcept
+    {
+      for(; __update != 0; --__update)
+       {
+          auto __err = sem_post(&_M_semaphore);
+          if (__err)
+            std::terminate();
+       }
+    }
+
+    bool
+    _M_try_acquire_until_impl(const chrono::time_point<__clock_t>& __atime)
+      noexcept
+    {
+
+      auto __s = chrono::time_point_cast<chrono::seconds>(__atime);
+      auto __ns = chrono::duration_cast<chrono::nanoseconds>(__atime - __s);
+
+      struct timespec __ts =
+      {
+       static_cast<std::time_t>(__s.time_since_epoch().count()),
+       static_cast<long>(__ns.count())
+      };
+
+      for (;;)
+       {
+         auto __err = sem_timedwait(&_M_semaphore, &__ts);
+         if (__err && (errno == EINTR))
+             continue;
+         else if (__err && (errno == ETIMEDOUT))
+             return false;
+         else if (__err && (errno == EINVAL))
+             return false; // caller supplied an invalid __atime
+         else if (__err)
+             std::terminate();
+         else
+           break;

It looks like this would be simpler with just one check for __err:

      auto __err = sem_timedwait(&_M_semaphore, &__ts);
      if (__err)
        {
          if (errno == EINTR)
            continue;
          if (errno == ETIMEDOUT || errno == EINVAL)
            return false;
          else
            std::terminate();
        }
      else
        break;

Hopefully the compiler wouldn't actually keep testing __err but I think
it reads better this way anyway.


+       }
+      return true;
+    }
+
+    template<typename _Clock, typename _Duration>
+      bool
+      _M_try_acquire_until(const chrono::time_point<_Clock,
+                          _Duration>& __atime) noexcept
+      {
+       if constexpr (std::is_same<__clock_t, _Clock>::value)

is_same_v (it's not just stylistic, the variable template is faster to
compile than the class template).

+         {
+           return _M_try_acquire_until_impl(__atime);
+         }
+       else
+         {
+           const typename _Clock::time_point __c_entry = _Clock::now();
+           const __clock_t __s_entry = __clock_t::now();
+           const auto __delta = __atime - __c_entry;
+           const auto __s_atime = __s_entry + __delta;
+           if (_M_try_acquire_until_impl(__s_atime))
+             return true;
+
+           // We got a timeout when measured against __clock_t but
+           // we need to check against the caller-supplied clock
+           // to tell whether we should return a timeout.
+           return (_Clock::now() < __atime);
+         }
+      }
+
+    template<typename _Rep, typename _Period>
+      _GLIBCXX_ALWAYS_INLINE bool
+      _M_try_acquire_for(const chrono::duration<_Rep, _Period>& __rtime)
+       noexcept
+      { return _M_try_acquire_until(__clock_t::now() + __rtime); }
+
+    private:
+      sem_t _M_semaphore;
+    };
+#endif // _GLIBCXX_HAVE_POSIX_SEMAPHORE
+
+    template<typename _Tp>
+      struct __atomic_semaphore
+      {
+       static_assert(std::is_integral_v<_Tp>);
+       static_assert(__gnu_cxx::__int_traits<_Tp>::__max
+                       <= __gnu_cxx::__int_traits<ptrdiff_t>::__max);
+       static constexpr ptrdiff_t _S_max = __gnu_cxx::__int_traits<_Tp>::__max;
+
+       explicit __atomic_semaphore(_Tp __count) noexcept
+         : _M_counter(__count)
+       {
+         __glibcxx_assert(__count >= 0 && __count <= _S_max);
+       }
+
+       __atomic_semaphore(const __atomic_semaphore&) = delete;
+       __atomic_semaphore& operator=(const __atomic_semaphore&) = delete;
+
+       _GLIBCXX_ALWAYS_INLINE void
+       _M_acquire() noexcept
+       {
+         auto const __pred = [this]
+           {
+             auto __old = __atomic_impl::load(&this->_M_counter,
+                             memory_order::acquire);
+             if (__old == 0)
+               return false;
+             return __atomic_impl::compare_exchange_strong(&this->_M_counter,
+                       __old, __old - 1,
+                       memory_order::acquire,
+                       memory_order::release);
+           };
+         auto __old = __atomic_impl::load(&_M_counter, memory_order_relaxed);
+         __atomic_wait(&_M_counter, __old, __pred);

Qualify to prevent ADL.


--- /dev/null
+++ b/libstdc++-v3/include/std/latch
@@ -0,0 +1,91 @@
+// <latch> -*- C++ -*-
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.       This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.         See the
+// GNU General Public License for more details.
+
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively.     If not, see
+// <http://www.gnu.org/licenses/>.
+
+/** @file include/latch
+ *  This is a Standard C++ Library header.
+ */
+
+#ifndef _GLIBCXX_LATCH
+#define _GLIBCXX_LATCH
+
+#pragma GCC system_header
+
+#if __cplusplus > 201703L
+#define __cpp_lib_latch 201907L
+
+#include <bits/atomic_base.h>
+#include <ext/numeric_traits.h>
+
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  class latch
+  {
+  public:
+    static constexpr ptrdiff_t
+    max() noexcept
+    { return __gnu_cxx::__int_traits<ptrdiff_t>::__max; }
+
+    constexpr explicit latch(ptrdiff_t __expected) noexcept
+      : _M_a(__expected) { }
+
+    ~latch() = default;
+    latch(const latch&) = delete;
+    latch& operator=(const latch&) = delete;
+
+    _GLIBCXX_ALWAYS_INLINE void
+    count_down(ptrdiff_t __update = 1)
+    {
+      auto const __old = __atomic_impl::fetch_sub(&_M_a,
+                                   __update, memory_order::release);
+      if (__old == __update)
+       __atomic_impl::notify_all(&_M_a);
+    }
+
+    _GLIBCXX_ALWAYS_INLINE bool
+    try_wait() const noexcept
+    { return __atomic_impl::load(&_M_a, memory_order::acquire) == 0; }
+
+    _GLIBCXX_ALWAYS_INLINE void
+    wait() const noexcept
+    {
+      auto const __old = __atomic_impl::load(&_M_a, memory_order::acquire);
+      __atomic_wait(&_M_a, __old, [this] { return this->try_wait(); });

Qualify to prevent ADL.

+    }
+
+    _GLIBCXX_ALWAYS_INLINE void
+    arrive_and_wait(ptrdiff_t __update = 1) noexcept
+    {
+      count_down(__update);
+      wait();
+    }
+
+  private:
+    alignas(__alignof__(ptrdiff_t)) ptrdiff_t _M_a;
+  };
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+#endif // __cplusplus > 201703L
+#endif // _GLIBCXX_LATCH

Reply via email to