[libc++] Split a few utilities out of __threading_support

[ldionne]

Summary

This change is the basis for a further refactoring where I'm going to
split up the various implementations we have in __threading_support to
make that code easier to understand.

Note that I had to make __convert_to_timespec a template to break
circular dependencies. Concretely, we never seem to use it with anything
other than ::timespec, but I am wary of hardcoding that assumption as
part of this change, since I suspect there's a reason for going through
these hoops in the first place.

[• Quuxplusone]

Looks plausible to me, FWIW, but nits.

libcxx/include/__thread/convert_to_timespec.h

27 ↗

(On Diff #398634)

In making this a template, you drive-by removed the inline keyword. I thought we weren't doing that. :)
I have a hunch that this helper belongs in __chrono/ not __thread/, but that's not a blocker.

libcxx/include/module.modulemap

906–908

You aren't including any of these detail headers from the actual <thread>. I think you should (or else, if <thread> doesn't actually use them, then they're in the wrong detail directory).

libcxx/src/atomic.cpp

11–12

I'd prefer to just include the public <thread>. I don't know if the Modules build will actually complain about including a private header like this from src/, but I just think it's better hygiene if src/ uses only the public headers.

(Pre-existing: At some point I'd love to find out why some src/*.cpp do e.g. #include "memory" instead of #include <memory>, and whether it'd be safe to change that.)

[ldionne]

Address review comments

libcxx/include/__thread/convert_to_timespec.h

27 ↗

(On Diff #398634)

In making this a template, you drive-by removed the inline keyword. I thought we weren't doing that. :)

You're right -- I removed it because templates implicitly have the right linkage, but I forgot about the fact that it can mess up optimizations (and I'm still working on figuring out the regression I mentioned elsewhere). I'll put inline back, good catch.

I have a hunch that this helper belongs in __chrono/ not __thread/, but that's not a blocker.

Yeah, I guess you're right, I'll move it there.

libcxx/src/atomic.cpp

11–12

I'd prefer to just include the public <thread>.

Done.

(Pre-existing: At some point I'd love to find out why some src/*.cpp do e.g. #include "memory" instead of #include <memory>, and whether it'd be safe to change that.)

I'm pretty sure it's safe to change that, it's just historical inconsistency.

[ldionne]

Rebase onto <chrono> modularization.

[• Quuxplusone]

LGTM FWIW.

libcxx/include/__chrono/convert_to_timespec.h

27

In fact __libcpp_timespec_t is unconditionally a typedef for ::timespec; so could we just use ::timespec here?
The template solution isn't too bad, but it still seems like it's working around a problem that shouldn't have been a problem in the first place.

libcxx/include/module.modulemap

370

[ldionne]

libcxx/include/__chrono/convert_to_timespec.h

27

As I said in the commit message:

Concretely, we never seem to use it with anything other than ::timespec, but I am wary of hardcoding that assumption as part of this change, since I suspect there's a reason for going through these hoops in the first place.

I can refactor that in a subsequent patch so that any fallout is separate from this one.

[ldionne]

Fix modular CI

[ldionne]

Rebase onto main, CI should be fixed.

[miyuki]

@ldionne, the reason for using __libcpp_timespec_t is the _LIBCPP_HAS_THREAD_API_EXTERNAL configuration. In that case __libcpp_timespec_t is defined in the __external_threading header.

[ldionne]

In D116944#3254049, @miyuki wrote:

@ldionne, the reason for using __libcpp_timespec_t is the _LIBCPP_HAS_THREAD_API_EXTERNAL configuration. In that case __libcpp_timespec_t is defined in the __external_threading header.

There you go, I knew there was a reason. Thanks!

libcxx/include/CMakeLists.txt

set(files
   __bit_reference
   __bits
   __bsd_locale_defaults.h
   __bsd_locale_fallbacks.h
   __charconv/chars_format.h
   __charconv/from_chars_result.h
   __charconv/to_chars_result.h
   __chrono/calendar.h
+  __chrono/convert_to_timespec.h
   __chrono/duration.h
   __chrono/file_clock.h
   __chrono/high_resolution_clock.h
   __chrono/steady_clock.h
   __chrono/system_clock.h
   __chrono/time_point.h
   __compare/common_comparison_category.h
   __compare/compare_three_way.h
set(files
   __support/solaris/wchar.h
   __support/solaris/xlocale.h
   __support/win32/limits_msvc_win32.h
   __support/win32/locale_win32.h
   __support/xlocale/__nop_locale_mgmt.h
   __support/xlocale/__posix_l_fallback.h
   __support/xlocale/__strtonum_fallback.h
   __thread/poll_with_backoff.h
+  __thread/timed_backoff_policy.h
   __threading_support
   __tree
   __tuple
   __undef_macros
   __utility/as_const.h
   __utility/auto_cast.h
   __utility/cmp.h
   __utility/declval.h

libcxx/include/__chrono/convert_to_timespec.h

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef _LIBCPP___CHRONO_CONVERT_TO_TIMESPEC_H
+#define _LIBCPP___CHRONO_CONVERT_TO_TIMESPEC_H
+
+#include <__chrono/duration.h>
+#include <__config>
+#include <limits>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// Convert a nanoseconds duration to the given TimeSpec type, which must have
+// the same properties as std::timespec.
+template <class _TimeSpec>
+_LIBCPP_HIDE_FROM_ABI inline
+_TimeSpec __convert_to_timespec(const chrono::nanoseconds& __ns)
+{
+  using namespace chrono;
+  seconds __s = duration_cast<seconds>(__ns);
+  _TimeSpec __ts;
+  typedef decltype(__ts.tv_sec) __ts_sec;
+  const __ts_sec __ts_sec_max = numeric_limits<__ts_sec>::max();
+
+  if (__s.count() < __ts_sec_max)
+  {
+    __ts.tv_sec = static_cast<__ts_sec>(__s.count());
+    __ts.tv_nsec = static_cast<decltype(__ts.tv_nsec)>((__ns - __s).count());
+  }
+  else
+  {
+    __ts.tv_sec = __ts_sec_max;
+    __ts.tv_nsec = 999999999; // (10^9 - 1)
+  }
+
+  return __ts;
+}
+
+_LIBCPP_END_NAMESPACE_STD
+
+_LIBCPP_POP_MACROS
+
+#endif // _LIBCPP___CHRONO_CONVERT_TO_TIMESPEC_H

libcxx/include/__thread/timed_backoff_policy.h

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#ifndef _LIBCPP___THREAD_TIMED_BACKOFF_POLICY_H
+#define _LIBCPP___THREAD_TIMED_BACKOFF_POLICY_H
+
+#include <__config>
+
+#ifndef _LIBCPP_HAS_NO_THREADS
+
+#include <__threading_support>
+#include <chrono>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+struct __libcpp_timed_backoff_policy {
+  _LIBCPP_INLINE_VISIBILITY
+  bool operator()(chrono::nanoseconds __elapsed) const
+  {
+      if(__elapsed > chrono::milliseconds(128))
+          __libcpp_thread_sleep_for(chrono::milliseconds(8));
+      else if(__elapsed > chrono::microseconds(64))
+          __libcpp_thread_sleep_for(__elapsed / 2);
+      else if(__elapsed > chrono::microseconds(4))
+        __libcpp_thread_yield();
+      else
+        {} // poll
+      return false;
+  }
+};
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP_HAS_NO_THREADS
+
+#endif // _LIBCPP___THREAD_TIMED_BACKOFF_POLICY_H

libcxx/include/__threading_support

 #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS __attribute__((no_thread_safety_analysis))
 #else
 #define _LIBCPP_NO_THREAD_SAFETY_ANALYSIS
 #endif
 
 typedef ::timespec __libcpp_timespec_t;
 #endif // !defined(_LIBCPP_HAS_NO_THREADS)
 
-_LIBCPP_PUSH_MACROS
-#include <__undef_macros>
-
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if !defined(_LIBCPP_HAS_NO_THREADS)
 
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 // Mutex
 typedef pthread_mutex_t __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER

 _LIBCPP_THREAD_ABI_VISIBILITY
 void *__libcpp_tls_get(__libcpp_tls_key __key);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_tls_set(__libcpp_tls_key __key, void *__p);
 
 #endif // !defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 
-struct __libcpp_timed_backoff_policy {
-  _LIBCPP_INLINE_VISIBILITY
-  bool operator()(chrono::nanoseconds __elapsed) const
-  {
-      if(__elapsed > chrono::milliseconds(128))
-          __libcpp_thread_sleep_for(chrono::milliseconds(8));
-      else if(__elapsed > chrono::microseconds(64))
-          __libcpp_thread_sleep_for(__elapsed / 2);
-      else if(__elapsed > chrono::microseconds(4))
-        __libcpp_thread_yield();
-      else
-        {} // poll
-      return false;
-  }
-};
-
 #if (!defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
      defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL))
 
-
-namespace __thread_detail {
-
-_LIBCPP_HIDE_FROM_ABI inline
-__libcpp_timespec_t __convert_to_timespec(const chrono::nanoseconds& __ns)
-{
-  using namespace chrono;
-  seconds __s = duration_cast<seconds>(__ns);
-  __libcpp_timespec_t __ts;
-  typedef decltype(__ts.tv_sec) __ts_sec;
-  const __ts_sec __ts_sec_max = numeric_limits<__ts_sec>::max();
-
-  if (__s.count() < __ts_sec_max)
-  {
-    __ts.tv_sec = static_cast<__ts_sec>(__s.count());
-    __ts.tv_nsec = static_cast<decltype(__ts.tv_nsec)>((__ns - __s).count());
-  }
-  else
-  {
-    __ts.tv_sec = __ts_sec_max;
-    __ts.tv_nsec = 999999999; // (10^9 - 1)
-  }
-
-  return __ts;
-}
-
-} // namespace __thread_detail
-
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 
 _LIBCPP_HIDE_FROM_ABI inline
 int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m)
 {
   pthread_mutexattr_t attr;
   int __ec = pthread_mutexattr_init(&attr);
   if (__ec)

 void __libcpp_thread_yield()
 {
   sched_yield();
 }
 
 _LIBCPP_HIDE_FROM_ABI inline
 void __libcpp_thread_sleep_for(const chrono::nanoseconds& __ns)
 {
-   __libcpp_timespec_t __ts = __thread_detail::__convert_to_timespec(__ns);
+   __libcpp_timespec_t __ts = _VSTD::__convert_to_timespec<__libcpp_timespec_t>(__ns);
    while (nanosleep(&__ts, &__ts) == -1 && errno == EINTR);
 }
 
 // Thread local storage
 _LIBCPP_HIDE_FROM_ABI inline
 int __libcpp_tls_create(__libcpp_tls_key *__key, void (*__at_exit)(void *))
 {
   return pthread_key_create(__key, __at_exit);

 void __libcpp_thread_yield()
 {
   thrd_yield();
 }
 
 _LIBCPP_HIDE_FROM_ABI inline
 void __libcpp_thread_sleep_for(const chrono::nanoseconds& __ns)
 {
-   __libcpp_timespec_t __ts = __thread_detail::__convert_to_timespec(__ns);
+   __libcpp_timespec_t __ts = _VSTD::__convert_to_timespec<__libcpp_timespec_t>(__ns);
   thrd_sleep(&__ts, nullptr);
 }
 
 // Thread local storage
 _LIBCPP_HIDE_FROM_ABI inline
 int __libcpp_tls_create(__libcpp_tls_key *__key, void (*__at_exit)(void *))
 {
   return tss_create(__key, __at_exit) == thrd_success ? 0 : EINVAL;

 }
 
 } // namespace this_thread
 
 #endif // !_LIBCPP_HAS_NO_THREADS
 
 _LIBCPP_END_NAMESPACE_STD
 
-_LIBCPP_POP_MACROS
-
 #endif // _LIBCPP_THREADING_SUPPORT

libcxx/include/atomic

 
 }  // std
 
 */
 
 #include <__availability>
 #include <__config>
 #include <__thread/poll_with_backoff.h>
+#include <__thread/timed_backoff_policy.h>
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
 #include <type_traits>
 #include <version>
 
 #ifndef _LIBCPP_HAS_NO_THREADS
 # include <__threading_support>

libcxx/include/barrier

namespace std
   };
 
 }
 
 */
 
 #include <__availability>
 #include <__config>
+#include <__thread/timed_backoff_policy.h>
 #include <atomic>
 #ifndef _LIBCPP_HAS_NO_TREE_BARRIER
 # include <memory>
 #endif
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif

libcxx/include/chrono

 constexpr chrono::year                                  operator ""y(unsigned long long y) noexcept; // C++20
 }  // chrono_literals
 }  // literals
 
 }  // std
 */
 
 #include <__chrono/calendar.h>
+#include <__chrono/convert_to_timespec.h>
 #include <__chrono/duration.h>
 #include <__chrono/file_clock.h>
 #include <__chrono/high_resolution_clock.h>
 #include <__chrono/steady_clock.h>
 #include <__chrono/system_clock.h>
 #include <__chrono/time_point.h>
 #include <__config>
 #include <compare>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 #endif // _LIBCPP_CHRONO

libcxx/include/module.modulemap

module std [system] {
 
   }
   module chrono {
     header "chrono"
     export *
 
     module __chrono {
       module calendar              { private header "__chrono/calendar.h" }
+      module convert_to_timespec   { private header "__chrono/convert_to_timespec.h" }
       module duration              { private header "__chrono/duration.h" }
       module file_clock            { private header "__chrono/file_clock.h" }
       module high_resolution_clock { private header "__chrono/high_resolution_clock.h" }
       module steady_clock          { private header "__chrono/steady_clock.h" }
       module system_clock          { private header "__chrono/system_clock.h" }
       module time_point            { private header "__chrono/time_point.h" }
     }
   }
module system_error {
     header "system_error"
     export *
   }
   module thread {
     header "thread"
     export *
 
     module __thread {
-      module poll_with_backoff { private header "__thread/poll_with_backoff.h" }
+      module poll_with_backoff    { private header "__thread/poll_with_backoff.h" }
+      module timed_backoff_policy { private header "__thread/timed_backoff_policy.h" }
     }
   }
   module tuple {
     header "tuple"
     export *
   }
   module type_traits {
     header "type_traits"
     export functional.__functional.unwrap_ref

libcxx/include/semaphore

 using binary_semaphore = counting_semaphore<1>;
 
 }
 
 */
 
 #include <__availability>
 #include <__config>
+#include <__thread/timed_backoff_policy.h>
 #include <__threading_support>
 #include <atomic>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 

libcxx/include/thread

 
 */
 
 #include <__config>
 #include <__debug>
 #include <__functional_base>
 #include <__mutex_base>
 #include <__thread/poll_with_backoff.h>
+#include <__thread/timed_backoff_policy.h>
 #include <__threading_support>
 #include <__utility/forward.h>
 #include <chrono>
 #include <cstddef>
 #include <functional>
 #include <iosfwd>
 #include <memory>
 #include <system_error>

libcxx/src/atomic.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <__config>
 #ifndef _LIBCPP_HAS_NO_THREADS
 
-#include <climits>
 #include <atomic>
+#include <climits>
 #include <functional>
+#include <thread>
 
 #ifdef __linux__
 
 #include <unistd.h>
 #include <linux/futex.h>
 #include <sys/syscall.h>
 
 // libc++ uses SYS_futex as a universal syscall name. However, on 32 bit architectures

libcxx/test/libcxx/diagnostics/detail.headers/chrono/convert_to_timespec.module.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-build
+
+// WARNING: This test was generated by 'generate_private_header_tests.py'
+// and should not be edited manually.
+
+// expected-error@*:* {{use of private header from outside its module: '__chrono/convert_to_timespec.h'}}
+#include <__chrono/convert_to_timespec.h>

libcxx/test/libcxx/diagnostics/detail.headers/thread/timed_backoff_policy.module.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-build
+
+// WARNING: This test was generated by 'generate_private_header_tests.py'
+// and should not be edited manually.
+
+// expected-error@*:* {{use of private header from outside its module: '__thread/timed_backoff_policy.h'}}
+#include <__thread/timed_backoff_policy.h>