[PATCH] Add C++2a synchronization support

[Jonathan Wakely]

On 03/08/20 15:09 +0100, Jonathan Wakely wrote:
>On 05/06/20 17:29 -0700, Thomas Rodgers wrote:
>>diff --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h
>>new file mode 100644
>>index 00000000000..f0c4235d91c
>>--- /dev/null
>>+++ b/libstdc++-v3/include/bits/semaphore_base.h
>>@@ -0,0 +1,272 @@
>>+// -*- 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>
>>+
>>+#if defined _POSIX_SEMAPHORES  && __has_include(<semaphore.h>)
>>+#define _GLIBCXX_HAVE_POSIX_SEMAPHORE 1
>>+#include <semaphore.h>
>>+#endif
>>+
>>+#include <chrono>
>>+#include <type_traits>
>>+
>>+namespace std _GLIBCXX_VISIBILITY(default)
>>+{
>>+_GLIBCXX_BEGIN_NAMESPACE_VERSION
>>+
>>+#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
>>+  template<ptrdiff_t __least_max_value>
>>+    struct __platform_semaphore

Why is this a template?

It means we'll instantiate identical code for counting_semaphore<3>
and counting_semaphore<4>, but they're distinct types that will bloat
the binary.

Can't we just have a single __platform_semaphore type for all max
values, and just make sure we don't instantiate it for values larger
than SEM_VALUE_MAX?

>>+    {
>
>I think we want to delete the copy constructor and copy assignment
>operator for this type and __atomic_semaphore.
>
>>+      using __clock_t = chrono::system_clock;
>>+
>>+      explicit __platform_semaphore(ptrdiff_t __count) noexcept
>>+      {
>>+	static_assert( __least_max_value <= SEM_VALUE_MAX, "");

I think it would be useful for __platform_semaphore to define a static
constexpr member like:

     static constexpr ptrdiff_t _S_max = SEM_VALUE_MAX;

And then __atomic_semaphore could define:

     static constexpr _Tp _S_max = __gnu_cxx::__int_traits<_Tp>::__max;

And then the alias __semaphore_base could be defined as:

#if _GLIBCXX_HAVE_LINUX_FUTEX && ! _GLIBCXX_REQUIRE_POSIX_SEMAPHORE
   // Use futex if available and user didn't force use of POSIX:
   using __fast_semaphore_base = __atomic_semaphore<__platform_wait_t>;
#elif _GLIBCXX_HAVE_POSIX_SEMAPHORE
   // Otherwise, use POSIX if available:
   using __fast_semaphore_base = __platform_semaphore<SEM_VALUE_MAX>;
#else
// Otherwise, use atomics:
   using __fast_semaphore_base = __atomic_semaphore<ptrdiff_t>;
#endif

   template<ptrdiff_t __least_max_value>
     using __semaphore_base = conditional_t<
       __least_max_value <= __fast_semaphore_base::_S_max,
       __fast_semaphore_base, __atomic_semaphore<ptrdiff_t>;

Would that make sense?

Do the comments above reflect the intent?


>>+#ifdef _GLIBCXX_REQUIRE_POSIX_SEMAPHORE
>
>What is this case for? This macro seems to only exist for use by a
>single testcase.
>
>Is it supposed to be something users can set? If so, it needs to be
>documented as ABI-breaking and as implying that counting_semaphore
>cannot be use with counts higher than SEM_VALUE_MAX.
>
>>+  template<ptrdiff_t __least_max_value>
>>+    using __semaphore_base = __platform_semaphore<__least_max_value>;
>>+#else
>>+#  ifdef _GLIBCXX_HAVE_LINUX_FUTEX
>>+  template<ptrdiff_t __least_max_value>
>>+    using __semaphore_base = conditional_t<(
>>+			      __least_max_value >= 0
>
>This condition is redundant, we already know that counting_semaphore
>has checked the value is non-negative.
>
>>+				&& __least_max_value <= __detail::__platform_wait_max_value),
>>+			      __atomic_semaphore<__detail::__platform_wait_t>,
>>+			      __atomic_semaphore<ptrdiff_t>>;
>>+
>>+// __platform_semaphore
>>+#  elif defined _GLIBCXX_HAVE_POSIX_SEMAPHORE
>>+  template<ptrdiff_t __least_max_value>
>>+    using __semaphore_base = conditional_t<(
>>+			      __least_max_value >= 0
>
>Redundant condition again.
>
>>+				&& __least_max_value <= SEM_VALUE_MAX),
>>+			      __platform_semaphore<__least_max_value>,
>>+			      __atomic_semaphore<ptrdiff_t>>;
>>+#  else
>>+  template<ptrdiff_t __least_max_value>
>>+    using __semaphore_base = __atomic_semaphore<ptrdiff_t>;
>>+#  endif
>>+#endif
>>+
>>+_GLIBCXX_END_NAMESPACE_VERSION
>>+} // namespace std
>>+
>>+#endif