[libc++] Handle threads-related .cpp files like we do all other source files #71100
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e files Source files in libc++ are added to the CMake targets only if they are required by the configuration. We do this pretty consistently for all configurations like no-filesystem, no-random-device, etc. but we didn't do it for no-threads. This patch makes this consistent for no-threads, which is helpful in reducing the amount of work required to port libc++ to some platforms without threads. Indeed, with the previous approach, several threads-related source files would end up including headers that might fail to compile properly on some platforms. This issue is sidestepped entirely by making the approach for no-threads consistent with the other configurations.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesSource files in libc++ are added to the CMake targets only if they are required by the configuration. We do this pretty consistently for all configurations like no-filesystem, no-random-device, etc. but we didn't do it for no-threads. This patch makes this consistent for no-threads, which is helpful in reducing the amount of work required to port libc++ to some platforms without threads. Indeed, with the previous approach, several threads-related source files would end up including headers that might fail to compile properly on some platforms. This issue is sidestepped entirely by making the approach for no-threads consistent with the other configurations. Full diff: https://github.com/llvm/llvm-project/pull/71100.diff 11 Files Affected:
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index e57fbf1468acb2b..156dbe8a4c2f92e 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -4,13 +4,10 @@ set(LIBCXX_LIB_CMAKEFILES_DIR "${CMAKE_CURRENT_BINARY_DIR}${CMAKE_FILES_DIRECTOR
set(LIBCXX_SOURCES
algorithm.cpp
any.cpp
- atomic.cpp
- barrier.cpp
bind.cpp
+ call_once.cpp
charconv.cpp
chrono.cpp
- condition_variable.cpp
- condition_variable_destructor.cpp
error_category.cpp
exception.cpp
filesystem/filesystem_clock.cpp
@@ -18,7 +15,6 @@ set(LIBCXX_SOURCES
filesystem/path_parser.h
filesystem/path.cpp
functional.cpp
- future.cpp
hash.cpp
include/apple_availability.h
include/atomic_support.h
@@ -37,8 +33,6 @@ set(LIBCXX_SOURCES
legacy_pointer_safety.cpp
memory.cpp
memory_resource.cpp
- mutex.cpp
- mutex_destructor.cpp
new_handler.cpp
new_helpers.cpp
optional.cpp
@@ -47,7 +41,6 @@ set(LIBCXX_SOURCES
ryu/d2fixed.cpp
ryu/d2s.cpp
ryu/f2s.cpp
- shared_mutex.cpp
stdexcept.cpp
string.cpp
support/runtime/exception_fallback.ipp
@@ -62,7 +55,6 @@ set(LIBCXX_SOURCES
support/runtime/stdexcept_default.ipp
support/runtime/stdexcept_vcruntime.ipp
system_error.cpp
- thread.cpp
typeinfo.cpp
valarray.cpp
variant.cpp
@@ -70,6 +62,20 @@ set(LIBCXX_SOURCES
verbose_abort.cpp
)
+if (LIBCXX_ENABLE_THREADS)
+ list(APPEND LIBCXX_SOURCES
+ atomic.cpp
+ barrier.cpp
+ condition_variable_destructor.cpp
+ condition_variable.cpp
+ future.cpp
+ mutex_destructor.cpp
+ mutex.cpp
+ shared_mutex.cpp
+ thread.cpp
+ )
+endif()
+
if (LIBCXX_ENABLE_RANDOM_DEVICE)
list(APPEND LIBCXX_SOURCES
random.cpp
diff --git a/libcxx/src/atomic.cpp b/libcxx/src/atomic.cpp
index fc90a1f5b2ba04c..133774bf13c4b04 100644
--- a/libcxx/src/atomic.cpp
+++ b/libcxx/src/atomic.cpp
@@ -6,9 +6,6 @@
//
//===----------------------------------------------------------------------===//
-#include <__config>
-#ifndef _LIBCPP_HAS_NO_THREADS
-
#include <__thread/timed_backoff_policy.h>
#include <atomic>
#include <climits>
@@ -219,5 +216,3 @@ void __libcpp_atomic_wait(__cxx_atomic_contention_t const volatile* __location,
}
_LIBCPP_END_NAMESPACE_STD
-
-#endif //_LIBCPP_HAS_NO_THREADS
diff --git a/libcxx/src/barrier.cpp b/libcxx/src/barrier.cpp
index 8ce2c043cf81dbc..3242e0865bffe68 100644
--- a/libcxx/src/barrier.cpp
+++ b/libcxx/src/barrier.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-#include <__config>
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-
#include <barrier>
#include <thread>
@@ -93,5 +89,3 @@ void __destroy_barrier_algorithm_base(__barrier_algorithm_base* __barrier)
#endif // !defined(_LIBCPP_HAS_NO_TREE_BARRIER)
_LIBCPP_END_NAMESPACE_STD
-
-#endif //_LIBCPP_HAS_NO_THREADS
diff --git a/libcxx/src/call_once.cpp b/libcxx/src/call_once.cpp
new file mode 100644
index 000000000000000..352cdcccdee0f19
--- /dev/null
+++ b/libcxx/src/call_once.cpp
@@ -0,0 +1,72 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 <__mutex/once_flag.h>
+#include <__utility/exception_guard.h>
+
+#ifndef _LIBCPP_HAS_NO_THREADS
+# include <__threading_support>
+#endif
+
+#include "include/atomic_support.h"
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+// If dispatch_once_f ever handles C++ exceptions, and if one can get to it
+// without illegal macros (unexpected macros not beginning with _UpperCase or
+// __lowercase), and if it stops spinning waiting threads, then call_once should
+// call into dispatch_once_f instead of here. Relevant radar this code needs to
+// keep in sync with: 7741191.
+
+#ifndef _LIBCPP_HAS_NO_THREADS
+static constinit __libcpp_mutex_t mut = _LIBCPP_MUTEX_INITIALIZER;
+static constinit __libcpp_condvar_t cv = _LIBCPP_CONDVAR_INITIALIZER;
+#endif
+
+void __call_once(volatile once_flag::_State_type& flag, void* arg,
+ void (*func)(void*))
+{
+#if defined(_LIBCPP_HAS_NO_THREADS)
+
+ if (flag == once_flag::_Unset) {
+ auto guard = std::__make_exception_guard([&flag] { flag = once_flag::_Unset; });
+ flag = once_flag::_Pending;
+ func(arg);
+ flag = once_flag::_Complete;
+ guard.__complete();
+ }
+
+#else // !_LIBCPP_HAS_NO_THREADS
+
+ __libcpp_mutex_lock(&mut);
+ while (flag == once_flag::_Pending)
+ __libcpp_condvar_wait(&cv, &mut);
+ if (flag == once_flag::_Unset) {
+ auto guard = std::__make_exception_guard([&flag] {
+ __libcpp_mutex_lock(&mut);
+ __libcpp_relaxed_store(&flag, once_flag::_Unset);
+ __libcpp_mutex_unlock(&mut);
+ __libcpp_condvar_broadcast(&cv);
+ });
+
+ __libcpp_relaxed_store(&flag, once_flag::_Pending);
+ __libcpp_mutex_unlock(&mut);
+ func(arg);
+ __libcpp_mutex_lock(&mut);
+ __libcpp_atomic_store(&flag, once_flag::_Complete, _AO_Release);
+ __libcpp_mutex_unlock(&mut);
+ __libcpp_condvar_broadcast(&cv);
+ guard.__complete();
+ } else {
+ __libcpp_mutex_unlock(&mut);
+ }
+
+#endif // !_LIBCPP_HAS_NO_THREADS
+}
+
+_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/src/condition_variable.cpp b/libcxx/src/condition_variable.cpp
index 87ce1d8434f8594..33e19568b4744f1 100644
--- a/libcxx/src/condition_variable.cpp
+++ b/libcxx/src/condition_variable.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-#include <__config>
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-
#include <condition_variable>
#include <thread>
@@ -92,5 +88,3 @@ notify_all_at_thread_exit(condition_variable& cond, unique_lock<mutex> lk)
_LIBCPP_END_NAMESPACE_STD
_LIBCPP_POP_MACROS
-
-#endif // !_LIBCPP_HAS_NO_THREADS
diff --git a/libcxx/src/condition_variable_destructor.cpp b/libcxx/src/condition_variable_destructor.cpp
index 333face19d50cad..36113ab2c27b2bc 100644
--- a/libcxx/src/condition_variable_destructor.cpp
+++ b/libcxx/src/condition_variable_destructor.cpp
@@ -14,10 +14,8 @@
#include <__config>
#include <__threading_support>
-#if !defined(_LIBCPP_HAS_NO_THREADS)
-# if _LIBCPP_ABI_VERSION == 1 || !defined(_LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION)
-# define NEEDS_CONDVAR_DESTRUCTOR
-# endif
+#if _LIBCPP_ABI_VERSION == 1 || !defined(_LIBCPP_HAS_TRIVIAL_CONDVAR_DESTRUCTION)
+# define NEEDS_CONDVAR_DESTRUCTOR
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
diff --git a/libcxx/src/future.cpp b/libcxx/src/future.cpp
index 3383b506a2fab3d..0e3f7b12b859e4b 100644
--- a/libcxx/src/future.cpp
+++ b/libcxx/src/future.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-#include <__config>
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-
#include <future>
#include <string>
@@ -268,5 +264,3 @@ shared_future<void>::operator=(const shared_future& __rhs)
}
_LIBCPP_END_NAMESPACE_STD
-
-#endif // !_LIBCPP_HAS_NO_THREADS
diff --git a/libcxx/src/mutex.cpp b/libcxx/src/mutex.cpp
index b165b1bc9debfc5..fe7a970ee1c488d 100644
--- a/libcxx/src/mutex.cpp
+++ b/libcxx/src/mutex.cpp
@@ -14,10 +14,8 @@
#include "include/atomic_support.h"
-#ifndef _LIBCPP_HAS_NO_THREADS
-# if defined(__ELF__) && defined(_LIBCPP_LINK_PTHREAD_LIB)
-# pragma comment(lib, "pthread")
-# endif
+#if defined(__ELF__) && defined(_LIBCPP_LINK_PTHREAD_LIB)
+# pragma comment(lib, "pthread")
#endif
_LIBCPP_PUSH_MACROS
@@ -25,8 +23,6 @@ _LIBCPP_PUSH_MACROS
_LIBCPP_BEGIN_NAMESPACE_STD
-#ifndef _LIBCPP_HAS_NO_THREADS
-
// ~mutex is defined elsewhere
void
@@ -189,60 +185,6 @@ recursive_timed_mutex::unlock() noexcept
}
}
-#endif // !_LIBCPP_HAS_NO_THREADS
-
-// If dispatch_once_f ever handles C++ exceptions, and if one can get to it
-// without illegal macros (unexpected macros not beginning with _UpperCase or
-// __lowercase), and if it stops spinning waiting threads, then call_once should
-// call into dispatch_once_f instead of here. Relevant radar this code needs to
-// keep in sync with: 7741191.
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-static constinit __libcpp_mutex_t mut = _LIBCPP_MUTEX_INITIALIZER;
-static constinit __libcpp_condvar_t cv = _LIBCPP_CONDVAR_INITIALIZER;
-#endif
-
-void __call_once(volatile once_flag::_State_type& flag, void* arg,
- void (*func)(void*))
-{
-#if defined(_LIBCPP_HAS_NO_THREADS)
-
- if (flag == once_flag::_Unset) {
- auto guard = std::__make_exception_guard([&flag] { flag = once_flag::_Unset; });
- flag = once_flag::_Pending;
- func(arg);
- flag = once_flag::_Complete;
- guard.__complete();
- }
-
-#else // !_LIBCPP_HAS_NO_THREADS
-
- __libcpp_mutex_lock(&mut);
- while (flag == once_flag::_Pending)
- __libcpp_condvar_wait(&cv, &mut);
- if (flag == once_flag::_Unset) {
- auto guard = std::__make_exception_guard([&flag] {
- __libcpp_mutex_lock(&mut);
- __libcpp_relaxed_store(&flag, once_flag::_Unset);
- __libcpp_mutex_unlock(&mut);
- __libcpp_condvar_broadcast(&cv);
- });
-
- __libcpp_relaxed_store(&flag, once_flag::_Pending);
- __libcpp_mutex_unlock(&mut);
- func(arg);
- __libcpp_mutex_lock(&mut);
- __libcpp_atomic_store(&flag, once_flag::_Complete, _AO_Release);
- __libcpp_mutex_unlock(&mut);
- __libcpp_condvar_broadcast(&cv);
- guard.__complete();
- } else {
- __libcpp_mutex_unlock(&mut);
- }
-
-#endif // !_LIBCPP_HAS_NO_THREADS
-}
-
_LIBCPP_END_NAMESPACE_STD
_LIBCPP_POP_MACROS
diff --git a/libcxx/src/mutex_destructor.cpp b/libcxx/src/mutex_destructor.cpp
index d366a4e1b317093..6096e424e661be6 100644
--- a/libcxx/src/mutex_destructor.cpp
+++ b/libcxx/src/mutex_destructor.cpp
@@ -19,10 +19,8 @@
#include <__config>
#include <__threading_support>
-#if !defined(_LIBCPP_HAS_NO_THREADS)
-# if _LIBCPP_ABI_VERSION == 1 || !defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)
-# define NEEDS_MUTEX_DESTRUCTOR
-# endif
+#if _LIBCPP_ABI_VERSION == 1 || !defined(_LIBCPP_HAS_TRIVIAL_MUTEX_DESTRUCTION)
+# define NEEDS_MUTEX_DESTRUCTOR
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -45,6 +43,6 @@ mutex::~mutex() noexcept
{
__libcpp_mutex_destroy(&__m_);
}
+#endif // !NEEDS_MUTEX_DESTRUCTOR
-#endif // !_LIBCPP_HAS_NO_THREADS
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/src/shared_mutex.cpp b/libcxx/src/shared_mutex.cpp
index 49a51e02712276f..1a346dda027f8e0 100644
--- a/libcxx/src/shared_mutex.cpp
+++ b/libcxx/src/shared_mutex.cpp
@@ -6,15 +6,11 @@
//
//===----------------------------------------------------------------------===//
-#include <__config>
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-
-# include <mutex>
-# include <shared_mutex>
-# if defined(__ELF__) && defined(_LIBCPP_LINK_PTHREAD_LIB)
-# pragma comment(lib, "pthread")
-# endif
+#include <mutex>
+#include <shared_mutex>
+#if defined(__ELF__) && defined(_LIBCPP_LINK_PTHREAD_LIB)
+# pragma comment(lib, "pthread")
+#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -95,5 +91,3 @@ bool shared_timed_mutex::try_lock_shared() { return __base_.try_lock_shared(); }
void shared_timed_mutex::unlock_shared() { return __base_.unlock_shared(); }
_LIBCPP_END_NAMESPACE_STD
-
-#endif // !_LIBCPP_HAS_NO_THREADS
diff --git a/libcxx/src/thread.cpp b/libcxx/src/thread.cpp
index 184b5ae8a187e09..289c457cd5a5cbd 100644
--- a/libcxx/src/thread.cpp
+++ b/libcxx/src/thread.cpp
@@ -6,10 +6,6 @@
//
//===----------------------------------------------------------------------===//
-#include <__config>
-
-#ifndef _LIBCPP_HAS_NO_THREADS
-
#include <__thread/poll_with_backoff.h>
#include <__thread/timed_backoff_policy.h>
#include <exception>
@@ -215,5 +211,3 @@ __thread_struct::__make_ready_at_thread_exit(__assoc_sub_state* __s)
}
_LIBCPP_END_NAMESPACE_STD
-
-#endif // !_LIBCPP_HAS_NO_THREADS
|
You can test this locally with the following command:git-clang-format --diff 29fd9bab2c9d04b90def77151961c02c940b15bb e18458bf7efd5dbbe4fe51a20a9b3d4c16b5e9d9 -- libcxx/src/call_once.cpp libcxx/src/shared_mutex.cppView the diff from clang-format here.diff --git a/libcxx/src/call_once.cpp b/libcxx/src/call_once.cpp
index 352cdcccdee0..b596518a6540 100644
--- a/libcxx/src/call_once.cpp
+++ b/libcxx/src/call_once.cpp
@@ -24,47 +24,45 @@ _LIBCPP_BEGIN_NAMESPACE_STD
// keep in sync with: 7741191.
#ifndef _LIBCPP_HAS_NO_THREADS
-static constinit __libcpp_mutex_t mut = _LIBCPP_MUTEX_INITIALIZER;
+static constinit __libcpp_mutex_t mut = _LIBCPP_MUTEX_INITIALIZER;
static constinit __libcpp_condvar_t cv = _LIBCPP_CONDVAR_INITIALIZER;
#endif
-void __call_once(volatile once_flag::_State_type& flag, void* arg,
- void (*func)(void*))
-{
+void __call_once(volatile once_flag::_State_type& flag, void* arg, void (*func)(void*)) {
#if defined(_LIBCPP_HAS_NO_THREADS)
- if (flag == once_flag::_Unset) {
- auto guard = std::__make_exception_guard([&flag] { flag = once_flag::_Unset; });
- flag = once_flag::_Pending;
- func(arg);
- flag = once_flag::_Complete;
- guard.__complete();
- }
+ if (flag == once_flag::_Unset) {
+ auto guard = std::__make_exception_guard([&flag] { flag = once_flag::_Unset; });
+ flag = once_flag::_Pending;
+ func(arg);
+ flag = once_flag::_Complete;
+ guard.__complete();
+ }
#else // !_LIBCPP_HAS_NO_THREADS
- __libcpp_mutex_lock(&mut);
- while (flag == once_flag::_Pending)
- __libcpp_condvar_wait(&cv, &mut);
- if (flag == once_flag::_Unset) {
- auto guard = std::__make_exception_guard([&flag] {
- __libcpp_mutex_lock(&mut);
- __libcpp_relaxed_store(&flag, once_flag::_Unset);
- __libcpp_mutex_unlock(&mut);
- __libcpp_condvar_broadcast(&cv);
- });
+ __libcpp_mutex_lock(&mut);
+ while (flag == once_flag::_Pending)
+ __libcpp_condvar_wait(&cv, &mut);
+ if (flag == once_flag::_Unset) {
+ auto guard = std::__make_exception_guard([&flag] {
+ __libcpp_mutex_lock(&mut);
+ __libcpp_relaxed_store(&flag, once_flag::_Unset);
+ __libcpp_mutex_unlock(&mut);
+ __libcpp_condvar_broadcast(&cv);
+ });
- __libcpp_relaxed_store(&flag, once_flag::_Pending);
- __libcpp_mutex_unlock(&mut);
- func(arg);
- __libcpp_mutex_lock(&mut);
- __libcpp_atomic_store(&flag, once_flag::_Complete, _AO_Release);
- __libcpp_mutex_unlock(&mut);
- __libcpp_condvar_broadcast(&cv);
- guard.__complete();
- } else {
- __libcpp_mutex_unlock(&mut);
- }
+ __libcpp_relaxed_store(&flag, once_flag::_Pending);
+ __libcpp_mutex_unlock(&mut);
+ func(arg);
+ __libcpp_mutex_lock(&mut);
+ __libcpp_atomic_store(&flag, once_flag::_Complete, _AO_Release);
+ __libcpp_mutex_unlock(&mut);
+ __libcpp_condvar_broadcast(&cv);
+ guard.__complete();
+ } else {
+ __libcpp_mutex_unlock(&mut);
+ }
#endif // !_LIBCPP_HAS_NO_THREADS
}
|
|
@tru Is it possible that there's a slight bug in how the For now I'll ship this and then clang-format |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Source files in libc++ are added to the CMake targets only if they are required by the configuration. We do this pretty consistently for all configurations like no-filesystem, no-random-device, etc. but we didn't do it for no-threads. This patch makes this consistent for no-threads, which is helpful in reducing the amount of work required to port libc++ to some platforms without threads.
Indeed, with the previous approach, several threads-related source files would end up including headers that might fail to compile properly on some platforms. This issue is sidestepped entirely by making the approach for no-threads consistent with the other configurations.