https://gcc.gnu.org/g:01670a4095791733e0389acead832e3da757c9d7

commit r15-3639-g01670a4095791733e0389acead832e3da757c9d7
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Mon Sep 2 12:29:04 2024 +0100

    libstdc++: Refactor loops in std::__platform_semaphore
    
    Refactor the loops to all use the same form, and to not need explicit
    'break' or 'continue' jumps. This also avoids a -Wunused-variable
    warning with -Wsystem-headers.
    
    Also fix a bug for absolute timeouts specified with a time that isn't
    implicitly convertible to __clock_t::time_point, e.g. one with a higher
    resolution such as picoseconds. Use chrono::ceil to round up to the next
    time point representable by the clock.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/semaphore_base.h (__platform_semaphore): Refactor
            loops to all use similar forms.
            (__platform_semaphore::_M_try_acquire_until): Use chrono::ceil
            to explicitly convert to __clock_t::time_point.
            * testsuite/30_threads/semaphore/try_acquire_for.cc: Check that
            using a very high resolution timeout compiles.
            * testsuite/30_threads/semaphore/platform_try_acquire_for.cc:
            New test.

Diff:
---
 libstdc++-v3/include/bits/semaphore_base.h         | 58 ++++++++--------------
 .../semaphore/platform_try_acquire_for.cc          |  7 +++
 .../30_threads/semaphore/try_acquire_for.cc        | 13 +++++
 3 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/libstdc++-v3/include/bits/semaphore_base.h 
b/libstdc++-v3/include/bits/semaphore_base.h
index 44a68645e477..2b19c9c6c6ac 100644
--- a/libstdc++-v3/include/bits/semaphore_base.h
+++ b/libstdc++-v3/include/bits/semaphore_base.h
@@ -73,52 +73,37 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     _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;
-       }
+      while (sem_wait(&_M_semaphore))
+       if (errno != EINTR)
+         std::__terminate();
     }
 
     _GLIBCXX_ALWAYS_INLINE bool
     _M_try_acquire() noexcept
     {
-      for (;;)
+      while (sem_trywait(&_M_semaphore))
        {
-         auto __err = sem_trywait(&_M_semaphore);
-         if (__err && (errno == EINTR))
-           continue;
-         else if (__err && (errno == EAGAIN))
+         if (errno == EAGAIN) // already locked
            return false;
-         else if (__err)
+         else if (errno != EINTR)
            std::__terminate();
-         else
-           break;
+         // else got EINTR so retry
        }
       return true;
     }
 
     _GLIBCXX_ALWAYS_INLINE void
-    _M_release(std::ptrdiff_t __update) noexcept
+    _M_release(ptrdiff_t __update) noexcept
     {
       for(; __update != 0; --__update)
-       {
-          auto __err = sem_post(&_M_semaphore);
-          if (__err)
-            std::__terminate();
-       }
+       if (sem_post(&_M_semaphore))
+         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);
 
@@ -128,19 +113,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
        static_cast<long>(__ns.count())
       };
 
-      for (;;)
+      while (sem_timedwait(&_M_semaphore, &__ts))
        {
-         if (auto __err = sem_timedwait(&_M_semaphore, &__ts))
-           {
-             if (errno == EINTR)
-               continue;
-             else if (errno == ETIMEDOUT || errno == EINVAL)
-               return false;
-             else
-               std::__terminate();
-           }
-         else
-           break;
+         if (errno == ETIMEDOUT)
+           return false;
+         else if (errno != EINTR)
+           std::__terminate();
        }
       return true;
     }
@@ -152,10 +130,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       {
        if constexpr (std::is_same_v<__clock_t, _Clock>)
          {
-           return _M_try_acquire_until_impl(__atime);
+           using _Dur = __clock_t::duration;
+           return _M_try_acquire_until_impl(chrono::ceil<_Dur>(__atime));
          }
        else
          {
+           // TODO: if _Clock is monotonic_clock we could use
+           // sem_clockwait with CLOCK_MONOTONIC.
+
            const typename _Clock::time_point __c_entry = _Clock::now();
            const auto __s_entry = __clock_t::now();
            const auto __delta = __atime - __c_entry;
diff --git 
a/libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc 
b/libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc
new file mode 100644
index 000000000000..bf6cd142bf01
--- /dev/null
+++ b/libstdc++-v3/testsuite/30_threads/semaphore/platform_try_acquire_for.cc
@@ -0,0 +1,7 @@
+// { dg-options "-D_GLIBCXX_USE_POSIX_SEMAPHORE" }
+// { dg-do run { target c++20 } }
+// { dg-additional-options "-pthread" { target pthread } }
+// { dg-require-gthreads "" }
+// { dg-add-options libatomic }
+
+#include "try_acquire_for.cc"
diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc 
b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc
index 53fe34d07530..330f8887e583 100644
--- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc
+++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_for.cc
@@ -78,8 +78,21 @@ void test02()
   b.wait(1);
 }
 
+void
+test03()
+{
+  using tick = std::chrono::system_clock::duration::period;
+  using halftick = std::ratio<tick::num, 2 * tick::den>;
+  std::chrono::duration<long long, halftick> timeout(1);
+  std::binary_semaphore s(1);
+
+  // Using higher resolution than chrono::system_clock should compile:
+  s.try_acquire_for(timeout);
+}
+
 int main()
 {
   test01();
   test02();
+  test03();
 }

Reply via email to