Skip to content

[libc++] Implement library support for MCF thread model #116550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

lhmouse
Copy link
Contributor

@lhmouse lhmouse commented Nov 17, 2024

This commit allows building libcxx and libcxxabi with

cmake -DLIBCXX_HAS_MCF_THREAD_API=ON
-DLIBCXXABI_HAS_MCF_THREAD_API=ON
...

This commit adds a new macro, _LIBCPP_HAS_THREAD_API_MCF, to indicate that the libraries have been built with MCF thread model.

These functions, and member functions of these classes, are implemented directly in libmcfgthread. The list is exhaustive. If someone wants to implement more, such as std::counting_semaphore and std::timed_mutex, please let me know:

  • __cxa_guard_acquire()
  • __cxa_guard_release()
  • __cxa_guard_abort()
  • std::mutex
  • std::call_once()
  • std::recursive_mutex
  • std::condition_variable
  • std::thread
  • std::this_thread::get_id()
  • std::this_thread::sleep_for()
  • std::this_thread::yield()

Reference: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=f036d759ecee538555fa8c6b11963e4033732463
Reference: https://github.com/lhmouse/mcfgthread/wiki/How-the-MCF-thread-model-works

@lhmouse lhmouse requested review from a team as code owners November 17, 2024 17:15
@llvmbot llvmbot added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. libc++abi libc++abi C++ Runtime Library. Not libc++. labels Nov 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2024

@llvm/pr-subscribers-libcxxabi

@llvm/pr-subscribers-libcxx

Author: LIU Hao (lhmouse)

Changes

This commit allows building libcxx and libcxxabi with

cmake -DLIBCXX_HAS_MCF_THREAD_API=ON
-DLIBCXXABI_HAS_MCF_THREAD_API=ON
...

This commit adds a new macro, _LIBCPP_HAS_THREAD_API_MCF, to indicate that the libraries have been built with MCF thread model.

These functions, and member functions of these classes, are implemented directly in libmcfgthread. The list is exhaustive. If someone wants to implement more, such as std::counting_semaphore and std::timed_mutex, please let me know:

  • __cxa_guard_acquire()
  • __cxa_guard_release()
  • __cxa_guard_abort()
  • std::mutex
  • std::call_once()
  • std::recursive_mutex
  • std::condition_variable
  • std::thread
  • std::this_thread::get_id()
  • std::this_thread::sleep_for()
  • std::this_thread::yield()

Reference: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=f036d759ecee538555fa8c6b11963e4033732463
Reference: https://github.com/lhmouse/mcfgthread/wiki/How-the-MCF-thread-model-works


Full diff: https://github.com/llvm/llvm-project/pull/116550.diff

11 Files Affected:

  • (modified) libcxx/CMakeLists.txt (+24-1)
  • (modified) libcxx/docs/DesignDocs/ThreadingSupportAPI.rst (+4)
  • (modified) libcxx/include/__config (+4-2)
  • (modified) libcxx/include/__config_site.in (+1)
  • (modified) libcxx/include/__thread/support.h (+2)
  • (modified) libcxx/src/CMakeLists.txt (+1-1)
  • (modified) libcxxabi/CMakeLists.txt (+28)
  • (modified) libcxxabi/src/CMakeLists.txt (+4)
  • (modified) libcxxabi/src/cxa_guard_impl.h (+47-1)
  • (modified) libcxxabi/src/cxa_thread_atexit.cpp (+11-6)
  • (modified) llvm/utils/gn/secondary/libcxx/include/BUILD.gn (+1)
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index abe12c2805a7cf..616b31dd79140e 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -295,6 +295,7 @@ endif()
 option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library" OFF)
 option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
 option(LIBCXX_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
+option(LIBCXX_HAS_MCF_THREAD_API "Ignore auto-detection and force use of mcfgthread API" OFF)
 option(LIBCXX_HAS_EXTERNAL_THREAD_API
   "Build libc++ with an externalized threading API.
    This option may only be set to ON when LIBCXX_ENABLE_THREADS=ON." OFF)
@@ -342,7 +343,10 @@ if(NOT LIBCXX_ENABLE_THREADS)
     message(FATAL_ERROR "LIBCXX_HAS_WIN32_THREAD_API can only be set to ON"
                         " when LIBCXX_ENABLE_THREADS is also set to ON.")
   endif()
-
+  if (LIBCXX_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "LIBCXX_HAS_MCF_THREAD_API can only be set to ON"
+                        " when LIBCXX_ENABLE_THREADS is also set to ON.")
+  endif()
 endif()
 
 if (LIBCXX_HAS_EXTERNAL_THREAD_API)
@@ -356,6 +360,11 @@ if (LIBCXX_HAS_EXTERNAL_THREAD_API)
                         " and LIBCXX_HAS_WIN32_THREAD_API cannot be both"
                         " set to ON at the same time.")
   endif()
+  if (LIBCXX_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "The options LIBCXX_HAS_EXTERNAL_THREAD_API"
+                        " and LIBCXX_HAS_MCF_THREAD_API cannot be both"
+                        " set to ON at the same time.")
+  endif()
 endif()
 
 if (LIBCXX_HAS_PTHREAD_API)
@@ -364,6 +373,19 @@ if (LIBCXX_HAS_PTHREAD_API)
                         " and LIBCXX_HAS_WIN32_THREAD_API cannot be both"
                         " set to ON at the same time.")
   endif()
+  if (LIBCXX_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "The options LIBCXX_HAS_PTHREAD_API"
+                        " and LIBCXX_HAS_MCF_THREAD_API cannot be both"
+                        " set to ON at the same time.")
+  endif()
+endif()
+
+if (LIBCXX_HAS_WIN32_THREAD_API)
+  if (LIBCXX_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "The options LIBCXX_HAS_WIN32_THREAD_API"
+                        " and LIBCXX_HAS_MCF_THREAD_API cannot be both"
+                        " set to ON at the same time.")
+  endif()
 endif()
 
 if (NOT LIBCXX_ENABLE_RTTI AND LIBCXX_ENABLE_EXCEPTIONS)
@@ -754,6 +776,7 @@ endif()
 config_define(${LIBCXX_HAS_PTHREAD_API} _LIBCPP_HAS_THREAD_API_PTHREAD)
 config_define(${LIBCXX_HAS_EXTERNAL_THREAD_API} _LIBCPP_HAS_THREAD_API_EXTERNAL)
 config_define(${LIBCXX_HAS_WIN32_THREAD_API} _LIBCPP_HAS_THREAD_API_WIN32)
+config_define(${LIBCXX_HAS_MCF_THREAD_API} _LIBCPP_HAS_THREAD_API_MCF)
 config_define(${LIBCXX_HAS_MUSL_LIBC} _LIBCPP_HAS_MUSL_LIBC)
 config_define_if(LIBCXX_NO_VCRUNTIME _LIBCPP_NO_VCRUNTIME)
 config_define(${LIBCXX_ENABLE_FILESYSTEM} _LIBCPP_HAS_FILESYSTEM)
diff --git a/libcxx/docs/DesignDocs/ThreadingSupportAPI.rst b/libcxx/docs/DesignDocs/ThreadingSupportAPI.rst
index d103c49e25952f..a8cf749fbaff08 100644
--- a/libcxx/docs/DesignDocs/ThreadingSupportAPI.rst
+++ b/libcxx/docs/DesignDocs/ThreadingSupportAPI.rst
@@ -65,3 +65,7 @@ Threading Configuration Macros
 **_LIBCPP_HAS_THREAD_API_WIN32**
   This macro is defined when libc++ should use Win32 threads to implement the
   internal threading API.
+
+**_LIBCPP_HAS_THREAD_API_MCF**
+  This macro is defined when libc++ should use the mcfgthread library to
+  implement the internal threading API.
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 9db00cd0c9fb93..5bee6efdc0456d 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -809,6 +809,7 @@ typedef __char32_t char32_t;
 #  if _LIBCPP_HAS_THREADS &&                                                                                           \
       !_LIBCPP_HAS_THREAD_API_PTHREAD &&                                                                               \
       !_LIBCPP_HAS_THREAD_API_WIN32 &&                                                                                 \
+      !_LIBCPP_HAS_THREAD_API_MCF &&                                                                                   \
       !_LIBCPP_HAS_THREAD_API_EXTERNAL
 
 #    if defined(__FreeBSD__) ||                                                                                        \
@@ -882,7 +883,7 @@ typedef __char32_t char32_t;
 // clang-format off
 #  if (_LIBCPP_HAS_THREAD_API_PTHREAD && defined(__GLIBC__)) ||                                                        \
       (_LIBCPP_HAS_THREAD_API_C11 && defined(__Fuchsia__)) ||                                                          \
-       _LIBCPP_HAS_THREAD_API_WIN32
+      _LIBCPP_HAS_THREAD_API_WIN32 || _LIBCPP_HAS_THREAD_API_MCF
 // clang-format on
 #    define _LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION 1
 #  else
@@ -897,7 +898,8 @@ typedef __char32_t char32_t;
 //
 // TODO(EricWF): This is potentially true for some pthread implementations
 // as well.
-#  if (_LIBCPP_HAS_THREAD_API_C11 && defined(__Fuchsia__)) || _LIBCPP_HAS_THREAD_API_WIN32
+#  if (_LIBCPP_HAS_THREAD_API_C11 && defined(__Fuchsia__)) ||                                                          \
+      _LIBCPP_HAS_THREAD_API_WIN32 || _LIBCPP_HAS_THREAD_API_MCF
 #    define _LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION 1
 #  else
 #    define _LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION 0
diff --git a/libcxx/include/__config_site.in b/libcxx/include/__config_site.in
index fc01aaf2d8746e..f5d2f0240c1003 100644
--- a/libcxx/include/__config_site.in
+++ b/libcxx/include/__config_site.in
@@ -20,6 +20,7 @@
 #cmakedefine01 _LIBCPP_HAS_THREAD_API_PTHREAD
 #cmakedefine01 _LIBCPP_HAS_THREAD_API_EXTERNAL
 #cmakedefine01 _LIBCPP_HAS_THREAD_API_WIN32
+#cmakedefine01 _LIBCPP_HAS_THREAD_API_MCF
 #define _LIBCPP_HAS_THREAD_API_C11 0 // FIXME: Is this guarding dead code?
 #cmakedefine _LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS
 #cmakedefine01 _LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS
diff --git a/libcxx/include/__thread/support.h b/libcxx/include/__thread/support.h
index 50a18daf2b685a..6b2e6851b20139 100644
--- a/libcxx/include/__thread/support.h
+++ b/libcxx/include/__thread/support.h
@@ -114,6 +114,8 @@ _LIBCPP_END_NAMESPACE_STD
 #    include <__thread/support/c11.h>
 #  elif _LIBCPP_HAS_THREAD_API_WIN32
 #    include <__thread/support/windows.h>
+#  elif defined(_LIBCPP_HAS_THREAD_API_MCF)
+#    include <mcfgthread/libcxx.h>
 #  else
 #    error "No threading API was selected"
 #  endif
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index cce8b8976f73c0..fa8e30741f1113 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -105,7 +105,7 @@ if(WIN32)
     support/win32/support.cpp
     )
 
-  if (NOT LIBCXX_HAS_PTHREAD_API)
+  if (LIBCXXABI_HAS_WIN32_THREAD_API)
     list(APPEND LIBCXX_SOURCES
       support/win32/thread_win32.cpp
       )
diff --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index da0e8b286cddc1..a876e4d74ae37e 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -56,6 +56,7 @@ option(LIBCXXABI_USE_COMPILER_RT "Use compiler-rt instead of libgcc" OFF)
 option(LIBCXXABI_ENABLE_THREADS "Build with threads enabled" ON)
 option(LIBCXXABI_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread API" OFF)
 option(LIBCXXABI_HAS_WIN32_THREAD_API "Ignore auto-detection and force use of win32 thread API" OFF)
+option(LIBCXXABI_HAS_MCF_THREAD_API "Ignore auto-detection and force use of mcfgthread API" OFF)
 option(LIBCXXABI_HAS_EXTERNAL_THREAD_API
   "Build libc++abi with an externalized threading API.
   This option may only be set to ON when LIBCXXABI_ENABLE_THREADS=ON." OFF)
@@ -318,6 +319,11 @@ if (NOT LIBCXXABI_ENABLE_THREADS)
                         " be set to ON when LIBCXXABI_ENABLE_THREADS"
                         " is also set to ON.")
   endif()
+  if (LIBCXXABI_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "LIBCXXABI_HAS_MCF_THREAD_API can only"
+                        " be set to ON when LIBCXXABI_ENABLE_THREADS"
+                        " is also set to ON.")
+  endif()
   add_definitions(-D_LIBCXXABI_HAS_NO_THREADS)
 endif()
 
@@ -332,6 +338,11 @@ if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
                         " and LIBCXXABI_HAS_WIN32_THREAD_API cannot be both"
                         " set to ON at the same time.")
   endif()
+  if (LIBCXXABI_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "The options LIBCXXABI_HAS_EXTERNAL_THREAD_API"
+                        " and LIBCXXABI_HAS_MCF_THREAD_API cannot be both"
+                        " set to ON at the same time.")
+  endif()
 endif()
 
 if (LIBCXXABI_HAS_PTHREAD_API)
@@ -340,6 +351,19 @@ if (LIBCXXABI_HAS_PTHREAD_API)
             "and LIBCXXABI_HAS_WIN32_THREAD_API cannot be both"
             "set to ON at the same time.")
   endif()
+  if (LIBCXXABI_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "The options LIBCXXABI_HAS_PTHREAD_API"
+            "and LIBCXXABI_HAS_MCF_THREAD_API cannot be both"
+            "set to ON at the same time.")
+  endif()
+endif()
+
+if (LIBCXXABI_HAS_WIN32_THREAD_API)
+  if (LIBCXXABI_HAS_MCF_THREAD_API)
+    message(FATAL_ERROR "The options LIBCXXABI_HAS_WIN32_THREAD_API"
+            "and LIBCXXABI_HAS_MCF_THREAD_API cannot be both"
+            "set to ON at the same time.")
+  endif()
 endif()
 
 if (LLVM_ENABLE_MODULES)
@@ -375,6 +399,10 @@ if (LIBCXXABI_HAS_EXTERNAL_THREAD_API)
   add_definitions(-D_LIBCPP_HAS_THREAD_API_EXTERNAL)
 endif()
 
+if (LIBCXXABI_HAS_MCF_THREAD_API)
+  add_definitions(-D_LIBCPP_HAS_THREAD_API_MCF)
+endif()
+
 if (MSVC)
   add_definitions(-D_CRT_SECURE_NO_WARNINGS)
 endif()
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index 84fe2784bec5ca..17c5d5e335f709 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -90,6 +90,10 @@ if (ANDROID AND ANDROID_PLATFORM_LEVEL LESS 21)
   list(APPEND LIBCXXABI_LIBRARIES android_support)
 endif()
 
+if (LIBCXXABI_HAS_MCF_THREAD_API)
+  list(APPEND LIBCXXABI_LIBRARIES mcfgthread ntdll)
+endif()
+
 # Setup flags.
 if (CXX_SUPPORTS_NOSTDLIBXX_FLAG)
   add_link_flags_if_supported(-nostdlib++)
diff --git a/libcxxabi/src/cxa_guard_impl.h b/libcxxabi/src/cxa_guard_impl.h
index 7b05bf32f3eda7..78d4fa5e1f20d7 100644
--- a/libcxxabi/src/cxa_guard_impl.h
+++ b/libcxxabi/src/cxa_guard_impl.h
@@ -68,6 +68,9 @@
 #  if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
 #    pragma comment(lib, "pthread")
 #  endif
+#  if _LIBCPP_HAS_THREAD_API_MCF
+#    include <mcfgthread/cxa.h>
+#  endif
 #endif
 
 #if defined(__clang__)
@@ -608,6 +611,39 @@ struct GuardObject {
   }
 };
 
+//===----------------------------------------------------------------------===//
+//                          GuardObject for MCF
+//===----------------------------------------------------------------------===//
+
+/// Forwards everything to libmcfgthread
+struct GuardObject_MCF {
+  GuardObject_MCF() = delete;
+  GuardObject_MCF(GuardObject_MCF const&) = delete;
+  GuardObject_MCF& operator=(GuardObject_MCF const&) = delete;
+
+private:
+  int64_t* guard_ptr;
+
+public:
+  explicit GuardObject_MCF(uint64_t* raw_guard_object)
+      : guard_ptr(reinterpret_cast<int64_t*>(raw_guard_object)) { }
+
+  /// Implements __cxa_guard_acquire.
+  AcquireResult cxa_guard_acquire() {
+    return static_cast<AcquireResult>(__MCF_cxa_guard_acquire(guard_ptr));
+  }
+
+  /// Implements __cxa_guard_release.
+  void cxa_guard_release() {
+    __MCF_cxa_guard_release(guard_ptr);
+  }
+
+  /// Implements __cxa_guard_abort.
+  void cxa_guard_abort() {
+    __MCF_cxa_guard_abort(guard_ptr);
+  }
+};
+
 //===----------------------------------------------------------------------===//
 //                          Convenience Classes
 //===----------------------------------------------------------------------===//
@@ -628,6 +664,9 @@ template <void (*Wait)(int*, int) = PlatformFutexWait, void (*Wake)(int*) = Plat
           uint32_t (*GetThreadIDArg)() = PlatformThreadID>
 using FutexGuard = GuardObject<InitByteFutex<Wait, Wake, GetThreadIDArg>>;
 
+/// MCFGuard - Forwards everything to libmcfgthread
+using MCFGuard = GuardObject_MCF;
+
 //===----------------------------------------------------------------------===//
 //
 //===----------------------------------------------------------------------===//
@@ -639,7 +678,7 @@ struct GlobalStatic {
 template <class T>
 _LIBCPP_CONSTINIT T GlobalStatic<T>::instance = {};
 
-enum class Implementation { NoThreads, GlobalMutex, Futex };
+enum class Implementation { NoThreads, GlobalMutex, Futex, MCF };
 
 template <Implementation Impl>
 struct SelectImplementation;
@@ -660,6 +699,11 @@ struct SelectImplementation<Implementation::Futex> {
   using type = FutexGuard<PlatformFutexWait, PlatformFutexWake, PlatformThreadID>;
 };
 
+template <>
+struct SelectImplementation<Implementation::MCF> {
+  using type = MCFGuard;
+};
+
 // TODO(EricWF): We should prefer the futex implementation when available. But
 // it should be done in a separate step from adding the implementation.
 constexpr Implementation CurrentImplementation =
@@ -667,6 +711,8 @@ constexpr Implementation CurrentImplementation =
     Implementation::NoThreads;
 #elif defined(_LIBCXXABI_USE_FUTEX)
     Implementation::Futex;
+#elif defined(_LIBCPP_HAS_THREAD_API_MCF)
+    Implementation::MCF;
 #else
     Implementation::GlobalMutex;
 #endif
diff --git a/libcxxabi/src/cxa_thread_atexit.cpp b/libcxxabi/src/cxa_thread_atexit.cpp
index 8546cfe48c3976..445eeb41aa6996 100644
--- a/libcxxabi/src/cxa_thread_atexit.cpp
+++ b/libcxxabi/src/cxa_thread_atexit.cpp
@@ -10,9 +10,12 @@
 #include "cxxabi.h"
 #include <__thread/support.h>
 #ifndef _LIBCXXABI_HAS_NO_THREADS
-#if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
-#pragma comment(lib, "pthread")
-#endif
+#  if defined(__ELF__) && defined(_LIBCXXABI_LINK_PTHREAD_LIB)
+#    pragma comment(lib, "pthread")
+#  endif
+#  if _LIBCPP_HAS_THREAD_API_MCF
+#    include <mcfgthread/cxa.h>
+#  endif
 #endif
 
 #include <stdlib.h>
@@ -22,14 +25,14 @@ namespace __cxxabiv1 {
   using Dtor = void(*)(void*);
 
   extern "C"
-#ifndef HAVE___CXA_THREAD_ATEXIT_IMPL
+#if !defined(HAVE___CXA_THREAD_ATEXIT_IMPL)
   // A weak symbol is used to detect this function's presence in the C library
   // at runtime, even if libc++ is built against an older libc
   _LIBCXXABI_WEAK
 #endif
   int __cxa_thread_atexit_impl(Dtor, void*, void*);
 
-#ifndef HAVE___CXA_THREAD_ATEXIT_IMPL
+#if !(_LIBCPP_HAS_THREAD_API_MCF || defined(HAVE___CXA_THREAD_ATEXIT_IMPL))
 
 namespace {
   // This implementation is used if the C library does not provide
@@ -109,7 +112,9 @@ namespace {
 extern "C" {
 
   _LIBCXXABI_FUNC_VIS int __cxa_thread_atexit(Dtor dtor, void* obj, void* dso_symbol) throw() {
-#ifdef HAVE___CXA_THREAD_ATEXIT_IMPL
+#if _LIBCPP_HAS_THREAD_API_MCF
+    return __MCF_cxa_thread_atexit(dtor, obj, dso_symbol);
+#elif defined(HAVE___CXA_THREAD_ATEXIT_IMPL)
     return __cxa_thread_atexit_impl(dtor, obj, dso_symbol);
 #else
     if (__cxa_thread_atexit_impl) {
diff --git a/llvm/utils/gn/secondary/libcxx/include/BUILD.gn b/llvm/utils/gn/secondary/libcxx/include/BUILD.gn
index 229a056c8afefd..8ab51e22711409 100644
--- a/llvm/utils/gn/secondary/libcxx/include/BUILD.gn
+++ b/llvm/utils/gn/secondary/libcxx/include/BUILD.gn
@@ -26,6 +26,7 @@ if (current_toolchain == default_toolchain) {
       "_LIBCPP_HAS_THREAD_API_PTHREAD=",
       "_LIBCPP_HAS_THREAD_API_EXTERNAL=",
       "_LIBCPP_HAS_THREAD_API_WIN32=",
+      "_LIBCPP_HAS_THREAD_API_MCF=",
       "_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS=",
       "_LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS=",
       "_LIBCPP_NO_VCRUNTIME=",

Copy link

github-actions bot commented Nov 17, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 1d4602070f96c9a6921d51a3b907f90cd2e3ae32 07bb5353f0d8ca6d4ece0d7d3876191a3648d300 --extensions ,h,cpp -- libcxx/include/__config libcxx/include/__thread/support.h libcxxabi/src/cxa_guard_impl.h libcxxabi/src/cxa_thread_atexit.cpp
View the diff from clang-format here.
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 5bee6efdc0..4ca457cf7e 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -898,8 +898,7 @@ typedef __char32_t char32_t;
 //
 // TODO(EricWF): This is potentially true for some pthread implementations
 // as well.
-#  if (_LIBCPP_HAS_THREAD_API_C11 && defined(__Fuchsia__)) ||                                                          \
-      _LIBCPP_HAS_THREAD_API_WIN32 || _LIBCPP_HAS_THREAD_API_MCF
+#  if (_LIBCPP_HAS_THREAD_API_C11 && defined(__Fuchsia__)) || _LIBCPP_HAS_THREAD_API_WIN32 || _LIBCPP_HAS_THREAD_API_MCF
 #    define _LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION 1
 #  else
 #    define _LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION 0
diff --git a/libcxxabi/src/cxa_guard_impl.h b/libcxxabi/src/cxa_guard_impl.h
index 5adc8edd37..c96592e253 100644
--- a/libcxxabi/src/cxa_guard_impl.h
+++ b/libcxxabi/src/cxa_guard_impl.h
@@ -627,13 +627,10 @@ private:
   int64_t* guard_ptr;
 
 public:
-  explicit GuardObject_MCF(uint64_t* raw_guard_object)
-      : guard_ptr(reinterpret_cast<int64_t*>(raw_guard_object)) { }
+  explicit GuardObject_MCF(uint64_t* raw_guard_object) : guard_ptr(reinterpret_cast<int64_t*>(raw_guard_object)) {}
 
   /// Implements __cxa_guard_acquire.
-  AcquireResult cxa_guard_acquire() {
-    return static_cast<AcquireResult>(__MCF_cxa_guard_acquire(guard_ptr));
-  }
+  AcquireResult cxa_guard_acquire() { return static_cast<AcquireResult>(__MCF_cxa_guard_acquire(guard_ptr)); }
 
   /// Implements __cxa_guard_release.
   void cxa_guard_release() { __MCF_cxa_guard_release(guard_ptr); }
diff --git a/libcxxabi/src/cxa_thread_atexit.cpp b/libcxxabi/src/cxa_thread_atexit.cpp
index 445eeb41aa..41ea88b6aa 100644
--- a/libcxxabi/src/cxa_thread_atexit.cpp
+++ b/libcxxabi/src/cxa_thread_atexit.cpp
@@ -26,15 +26,16 @@ namespace __cxxabiv1 {
 
   extern "C"
 #if !defined(HAVE___CXA_THREAD_ATEXIT_IMPL)
-  // A weak symbol is used to detect this function's presence in the C library
-  // at runtime, even if libc++ is built against an older libc
-  _LIBCXXABI_WEAK
+      // A weak symbol is used to detect this function's presence in the C library
+      // at runtime, even if libc++ is built against an older libc
+      _LIBCXXABI_WEAK
 #endif
-  int __cxa_thread_atexit_impl(Dtor, void*, void*);
+      int
+      __cxa_thread_atexit_impl(Dtor, void*, void*);
 
 #if !(_LIBCPP_HAS_THREAD_API_MCF || defined(HAVE___CXA_THREAD_ATEXIT_IMPL))
 
-namespace {
+  namespace {
   // This implementation is used if the C library does not provide
   // __cxa_thread_atexit_impl() for us.  It has a number of limitations that are
   // difficult to impossible to address without ..._impl():

@lhmouse lhmouse force-pushed the main branch 4 times, most recently from f561005 to 6717827 Compare November 17, 2024 19:24
@lhmouse
Copy link
Contributor Author

lhmouse commented Nov 18, 2024

Building with clang-cl fails with a strange error:

[1/314] Generate the mapping file for include-what-you-use
FAILED: include/c++/v1/libcxx.imp E:/lh_mouse/Desktop/libcxx-build/include/c++/v1/libcxx.imp
cd /D E:\lh_mouse\Desktop\libcxx-build\include
CreateProcess failed: The system cannot find the file specified.
[3/314] Copying CXX header __thread/support.hninja: fatal: ReadFile: The handle is invalid.

@lhmouse
Copy link
Contributor Author

lhmouse commented Nov 18, 2024

What's the limit of characters perline in llvm? I see some errors from clang-format but not all of them are about my changes. I usually hard-wrap lines around 80 (mostly before, sometimes after) however in llvm there are much longer lines, around 120 characters.

There was a test that timed which I have no idea at the moment: https://github.com/llvm/llvm-project/actions/runs/11884156644/job/33112784777?pr=116550#step:4:4140
(Is it about MSVC? then how does it run on self-hosted Linux?)

@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Nov 18, 2024

What's the limit of characters perline in llvm? I see some errors from clang-format but not all of them are about my changes. I usually hard-wrap lines around 80 (mostly before, sometimes after) however in llvm there are much longer lines, around 120 characters.

In libc++ the limit is 120. (Although the limit is 80 for some files under /llvm, which is inconsistent.)

There was a test that timed which I have no idea at the moment: https://github.com/llvm/llvm-project/actions/runs/11884156644/job/33112784777?pr=116550#step:4:4140
(Is it about MSVC? then how does it run on self-hosted Linux?)

This error doesn't always occur. It seems that the error is sometimes ignored when merging.

The test doesn't require MSVC. The name was because of that the test was cloned from MSVC STL's corresponding test.

@frederick-vs-ja
Copy link
Contributor

We can rebase the patch now as the test is (temporarily?) disabled for MSan by #116933.

This commit allows building libcxx and libcxxabi with

  cmake -DLIBCXX_HAS_MCF_THREAD_API=ON \
        -DLIBCXXABI_HAS_MCF_THREAD_API=ON \
        ...

This commit adds a new macro, `_LIBCPP_HAS_THREAD_API_MCF`, to indicate that
the libraries have been built with MCF thread model.

These functions, and member functions of these classes, are implemented
directly in libmcfgthread. The list is exhaustive. If someone wants to
implement more, such as `std::counting_semaphore` and `std::timed_mutex`,
please let me know:

  * `__cxa_guard_acquire()`
  * `__cxa_guard_release()`
  * `__cxa_guard_abort()`
  * `std::mutex`
  * `std::call_once()`
  * `std::recursive_mutex`
  * `std::condition_variable`
  * `std::thread`
  * `std::this_thread::get_id()`
  * `std::this_thread::sleep_for()`
  * `std::this_thread::yield()`

Reference: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=f036d759ecee538555fa8c6b11963e4033732463
Reference: https://github.com/lhmouse/mcfgthread/wiki/How-the-MCF-thread-model-works
Signed-off-by: LIU Hao <[email protected]>
@lhmouse
Copy link
Contributor Author

lhmouse commented Nov 22, 2024

Done now.

@ldionne
Copy link
Member

ldionne commented Nov 26, 2024

This is on my stack of reviews, I should get to it tomorrow.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a high-level, the first point of discussion IMO is how this new "backend" is integrated into libc++. As it stands, you do this:

#elif defined(_LIBCPP_HAS_THREAD_API_MCF)
#    include <mcfgthread/libcxx.h>

In other words, the code is pulled in from elsewhere (presumably https://github.com/lhmouse/mcfgthread/blob/master/mcfgthread/libcxx.h), and it's assumed to be available. Instead, I would expect the libc++ entry points to be defined in libc++ itself, and then for these functions to call into the MCF runtime as needed. What would you think of that approach instead? The reason why this is fairly important is that we normally don't include random "external" headers from libc++ (with the exception of __external_threading, which should be redesigned). This also means that libc++ isn't self-sufficient anymore -- it needs to pull in a header from another project in order to build in that configuration, which also means that we can't officially test (hence officially support) this configuration.

For all these reasons, I think it would be better if the libc++-specific API were contained inside libc++, and libc++ only called into MCF entry points defined in the appropriate DLL. To use this configuration, it would then be sufficient to

  1. Install the MCF runtime
  2. Build libc++ with LIBCXX_HAS_MCF_THREAD_API=ON

I'd also like to remind that contributions must be under the Apache license with the LLVM exception, so this must be kept in mind in case the libc++-specific part of the API is contributed here.

@lhmouse
Copy link
Contributor Author

lhmouse commented Nov 29, 2024

In other words, the code is pulled in from elsewhere (presumably https://github.com/lhmouse/mcfgthread/blob/master/mcfgthread/libcxx.h), and it's assumed to be available.

This is correct.

Instead, I would expect the libc++ entry points to be defined in libc++ itself, and then for these functions to call into the MCF runtime as needed. What would you think of that approach instead? The reason why this is fairly important is that we normally don't include random "external" headers from libc++ (with the exception of __external_threading, which should be redesigned). This also means that libc++ isn't self-sufficient anymore -- it needs to pull in a header from another project in order to build in that configuration, which also means that we can't officially test (hence officially support) this configuration.

Indeed, for GCC and libstdc++ it's an external library. However winpthreads is also not a system library. If, by definition, 'anything that exists in mingw-w64' is not an external library, I can try committing a replica into mingw-w64-libraries. This was mentioned some years ago but it has not been done.

Another option is below:

For all these reasons, I think it would be better if the libc++-specific API were contained inside libc++, and libc++ only called into MCF entry points defined in the appropriate DLL. To use this configuration, it would then be sufficient to

  1. Install the MCF runtime
  2. Build libc++ with LIBCXX_HAS_MCF_THREAD_API=ON

I'd also like to remind that contributions must be under the Apache license with the LLVM exception, so this must be kept in mind in case the libc++-specific part of the API is contributed here.

It looks to me that another option is to copy 'mcfgthread/libcxx.h' into libc++..? Not sure whether I got your idea.

All headers from mcfgthread are public domain and can be freely copied. (Trivia: Previously they were declared as public domain. However it's kinda self-contradiction that the license applies to something it does not apply or vice versa. The new license says it applies to headers but headers can be used in public domain instead.)

@ldionne
Copy link
Member

ldionne commented Dec 4, 2024

It looks to me that another option is to copy 'mcfgthread/libcxx.h' into libc++..? Not sure whether I got your idea.

Without necessarily copying (due to licensing constraints), but yeah the idea would be to have the bridge-to-MFC inside libc++ instead of elsewhere.

All headers from mcfgthread are public domain and can be freely copied. (Trivia: Previously they were declared as public domain. However it's kinda self-contradiction that the license applies to something it does not apply or vice versa. The new license says it applies to headers but headers can be used in public domain instead.)

This isn't really the right forum for discussing licensing or copyright matters, but if you're able to contribute an implementation of the above under the LLVM license, that would be the preferred way of supporting that configuration. As I said before, what we basically don't want is to check-in code in libc++ that blindly includes external code that doesn't exist in libc++ -- since that makes such code dead as far as we're concerned (CI, etc). If you have questions about licensing, I think I would direct you towards [email protected].

@lhmouse
Copy link
Contributor Author

lhmouse commented Dec 5, 2024

This isn't really the right forum for discussing licensing or copyright matters, but if you're able to contribute an implementation of the above under the LLVM license, that would be the preferred way of supporting that configuration. As I said before, what we basically don't want is to check-in code in libc++ that blindly includes external code that doesn't exist in libc++ -- since that makes such code dead as far as we're concerned (CI, etc). If you have

Understood. Thank you.

@lhmouse lhmouse closed this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++abi libc++abi C++ Runtime Library. Not libc++. libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants