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
