t.p.northover updated this revision to Diff 348958.
t.p.northover added a comment.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, hiraditya.
Herald added a project: clang-tools-extra.
- Add `llvm::thread` for all potential platforms, allowing stack size to be
specified.
- `llvm::thread` based on the `Threading.inc` where possible, else
`std::thread`, else in-thread synchronous execution (with progressively more
failures).
- Remove `llvm_execute_on_thread*` in favour of this new `llvm::thread`.
I've managed to test it on all 3 major platforms, and the fallbacks by fiddling
`#if`s. So it should at least compile everywhere.
> Also this way llvm::thread users that don't need full-sized stacks can
> continue getting the Darwin default of smaller pages.
I'm not sure I've implemented this, since I've just made 8MB the Darwin default
here. The case I care about is LTOBackend via `ThreadPool`. Would you prefer me
to push an optional `StackSize` argument into `ThreadPool` too and make
LTOBackend specify it?
I kind of think this has come up often enough (it's at least the second time
I've had to fix it somewhere), and 8MB is small enough for anything running
Clang that it's better to make sure it doesn't happen again.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103165/new/
https://reviews.llvm.org/D103165
Files:
clang-tools-extra/clangd/support/Threading.cpp
clang/tools/libclang/CIndex.cpp
llvm/include/llvm/Support/CrashRecoveryContext.h
llvm/include/llvm/Support/Threading.h
llvm/include/llvm/Support/thread.h
llvm/lib/Support/CrashRecoveryContext.cpp
llvm/lib/Support/Threading.cpp
llvm/lib/Support/Unix/Threading.inc
llvm/lib/Support/Windows/Threading.inc
llvm/unittests/Support/Threading.cpp
Index: llvm/unittests/Support/Threading.cpp
===================================================================
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -63,7 +63,8 @@
ThreadFinished.notify();
};
- llvm::llvm_execute_on_thread_async(ThreadFunc);
+ llvm::thread Thread(ThreadFunc);
+ Thread.detach();
ASSERT_TRUE(ThreadStarted.wait());
ThreadAdvanced.notify();
ASSERT_TRUE(ThreadFinished.wait());
@@ -71,11 +72,23 @@
TEST(Threading, RunOnThreadSync) {
std::atomic_bool Executed(false);
- llvm::llvm_execute_on_thread(
+ llvm::thread Thread(
[](void *Arg) { *static_cast<std::atomic_bool *>(Arg) = true; },
&Executed);
+ Thread.join();
ASSERT_EQ(Executed, true);
}
+
+#if defined(__APPLE__)
+TEST(Threading, AppleStackSize) {
+ llvm::thread Thread([] {
+ volatile unsigned char Var[8 * 1024 * 1024 - 1024];
+ Var[0] = 0xff;
+ ASSERT_EQ(Var[0], 0xff);
+ });
+ Thread.join();
+}
+#endif
#endif
} // end anon namespace
Index: llvm/lib/Support/Windows/Threading.inc
===================================================================
--- llvm/lib/Support/Windows/Threading.inc
+++ llvm/lib/Support/Windows/Threading.inc
@@ -23,22 +23,10 @@
#undef MemoryFence
#endif
-static unsigned __stdcall threadFuncSync(void *Arg) {
- SyncThreadInfo *TI = static_cast<SyncThreadInfo *>(Arg);
- TI->UserFn(TI->UserData);
- return 0;
-}
-
-static unsigned __stdcall threadFuncAsync(void *Arg) {
- std::unique_ptr<AsyncThreadInfo> Info(static_cast<AsyncThreadInfo *>(Arg));
- (*Info)();
- return 0;
-}
-
-static void
-llvm_execute_on_thread_impl(unsigned (__stdcall *ThreadFunc)(void *), void *Arg,
- llvm::Optional<unsigned> StackSizeInBytes,
- JoiningPolicy JP) {
+HANDLE
+llvm::llvm_execute_on_thread_impl(unsigned(__stdcall *ThreadFunc)(void *),
+ void *Arg,
+ llvm::Optional<unsigned> StackSizeInBytes) {
HANDLE hThread = (HANDLE)::_beginthreadex(
NULL, StackSizeInBytes.getValueOr(0), ThreadFunc, Arg, 0, NULL);
@@ -46,11 +34,16 @@
ReportLastErrorFatal("_beginthreadex failed");
}
- if (JP == JoiningPolicy::Join) {
- if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
- ReportLastErrorFatal("WaitForSingleObject failed");
- }
+ return hThread;
+}
+
+void llvm::llvm_thread_join_impl(HANDLE hThread) {
+ if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
+ ReportLastErrorFatal("WaitForSingleObject failed");
}
+}
+
+void llvm::llvm_thread_detach_impl(HANDLE hThread) {
if (::CloseHandle(hThread) == FALSE) {
ReportLastErrorFatal("CloseHandle failed");
}
Index: llvm/lib/Support/Unix/Threading.inc
===================================================================
--- llvm/lib/Support/Unix/Threading.inc
+++ llvm/lib/Support/Unix/Threading.inc
@@ -48,22 +48,9 @@
#include <unistd.h> // For syscall()
#endif
-static void *threadFuncSync(void *Arg) {
- SyncThreadInfo *TI = static_cast<SyncThreadInfo *>(Arg);
- TI->UserFn(TI->UserData);
- return nullptr;
-}
-
-static void *threadFuncAsync(void *Arg) {
- std::unique_ptr<AsyncThreadInfo> Info(static_cast<AsyncThreadInfo *>(Arg));
- (*Info)();
- return nullptr;
-}
-
-static void
-llvm_execute_on_thread_impl(void *(*ThreadFunc)(void *), void *Arg,
- llvm::Optional<unsigned> StackSizeInBytes,
- JoiningPolicy JP) {
+pthread_t
+llvm::llvm_execute_on_thread_impl(void *(*ThreadFunc)(void *), void *Arg,
+ llvm::Optional<unsigned> StackSizeInBytes) {
int errnum;
// Construct the attributes object.
@@ -90,15 +77,22 @@
if ((errnum = ::pthread_create(&Thread, &Attr, ThreadFunc, Arg)) != 0)
ReportErrnumFatal("pthread_create failed", errnum);
- if (JP == JoiningPolicy::Join) {
- // Wait for the thread
- if ((errnum = ::pthread_join(Thread, nullptr)) != 0) {
- ReportErrnumFatal("pthread_join failed", errnum);
- }
- } else if (JP == JoiningPolicy::Detach) {
- if ((errnum = ::pthread_detach(Thread)) != 0) {
- ReportErrnumFatal("pthread_detach failed", errnum);
- }
+ return Thread;
+}
+
+void llvm::llvm_thread_detach_impl(pthread_t Thread) {
+ int errnum;
+
+ if ((errnum = ::pthread_detach(Thread)) != 0) {
+ ReportErrnumFatal("pthread_detach failed", errnum);
+ }
+}
+
+void llvm::llvm_thread_join_impl(pthread_t Thread) {
+ int errnum;
+
+ if ((errnum = ::pthread_join(Thread, nullptr)) != 0) {
+ ReportErrnumFatal("pthread_join failed", errnum);
}
}
Index: llvm/lib/Support/Threading.cpp
===================================================================
--- llvm/lib/Support/Threading.cpp
+++ llvm/lib/Support/Threading.cpp
@@ -11,6 +11,7 @@
//
//===----------------------------------------------------------------------===//
+#include "llvm/Support/thread.h"
#include "llvm/Support/Threading.h"
#include "llvm/ADT/Optional.h"
#include "llvm/Config/config.h"
@@ -38,13 +39,6 @@
#if LLVM_ENABLE_THREADS == 0 || \
(!defined(_WIN32) && !defined(HAVE_PTHREAD_H))
-// Support for non-Win32, non-pthread implementation.
-void llvm::llvm_execute_on_thread(void (*Fn)(void *), void *UserData,
- llvm::Optional<unsigned> StackSizeInBytes) {
- (void)StackSizeInBytes;
- Fn(UserData);
-}
-
uint64_t llvm::get_threadid() { return 0; }
uint32_t llvm::get_max_thread_name_length() { return 0; }
@@ -60,25 +54,6 @@
return 1;
}
-#if LLVM_ENABLE_THREADS == 0
-void llvm::llvm_execute_on_thread_async(
- llvm::unique_function<void()> Func,
- llvm::Optional<unsigned> StackSizeInBytes) {
- (void)Func;
- (void)StackSizeInBytes;
- report_fatal_error("Spawning a detached thread doesn't make sense with no "
- "threading support");
-}
-#else
-// Support for non-Win32, non-pthread implementation.
-void llvm::llvm_execute_on_thread_async(
- llvm::unique_function<void()> Func,
- llvm::Optional<unsigned> StackSizeInBytes) {
- (void)StackSizeInBytes;
- std::thread(std::move(Func)).detach();
-}
-#endif
-
#else
int computeHostNumHardwareThreads();
@@ -95,17 +70,6 @@
return std::min((unsigned)MaxThreadCount, ThreadsRequested);
}
-namespace {
-struct SyncThreadInfo {
- void (*UserFn)(void *);
- void *UserData;
-};
-
-using AsyncThreadInfo = llvm::unique_function<void()>;
-
-enum class JoiningPolicy { Join, Detach };
-} // namespace
-
// Include the platform-specific parts of this class.
#ifdef LLVM_ON_UNIX
#include "Unix/Threading.inc"
@@ -114,22 +78,6 @@
#include "Windows/Threading.inc"
#endif
-void llvm::llvm_execute_on_thread(void (*Fn)(void *), void *UserData,
- llvm::Optional<unsigned> StackSizeInBytes) {
-
- SyncThreadInfo Info = {Fn, UserData};
- llvm_execute_on_thread_impl(threadFuncSync, &Info, StackSizeInBytes,
- JoiningPolicy::Join);
-}
-
-void llvm::llvm_execute_on_thread_async(
- llvm::unique_function<void()> Func,
- llvm::Optional<unsigned> StackSizeInBytes) {
- llvm_execute_on_thread_impl(&threadFuncAsync,
- new AsyncThreadInfo(std::move(Func)),
- StackSizeInBytes, JoiningPolicy::Detach);
-}
-
#endif
Optional<ThreadPoolStrategy>
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===================================================================
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -12,6 +12,7 @@
#include "llvm/Support/ExitCodes.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Signals.h"
+#include "llvm/Support/thread.h"
#include "llvm/Support/ThreadLocal.h"
#include <mutex>
#include <setjmp.h>
@@ -500,10 +501,12 @@
unsigned RequestedStackSize) {
bool UseBackgroundPriority = hasThreadBackgroundPriority();
RunSafelyOnThreadInfo Info = { Fn, this, UseBackgroundPriority, false };
- llvm_execute_on_thread(RunSafelyOnThread_Dispatch, &Info,
- RequestedStackSize == 0
- ? llvm::None
- : llvm::Optional<unsigned>(RequestedStackSize));
+ llvm::thread Thread(RequestedStackSize == 0
+ ? llvm::None
+ : llvm::Optional<unsigned>(RequestedStackSize),
+ RunSafelyOnThread_Dispatch, &Info);
+ Thread.join();
+
if (CrashRecoveryContextImpl *CRC = (CrashRecoveryContextImpl *)Impl)
CRC->setSwitchedThread();
return Info.Result;
Index: llvm/include/llvm/Support/thread.h
===================================================================
--- llvm/include/llvm/Support/thread.h
+++ llvm/include/llvm/Support/thread.h
@@ -16,16 +16,215 @@
#ifndef LLVM_SUPPORT_THREAD_H
#define LLVM_SUPPORT_THREAD_H
+#include "llvm/ADT/Optional.h"
#include "llvm/Config/llvm-config.h"
+#ifdef _WIN32
+#include "llvm/Support/Windows/WindowsSupport.h"
+#endif
+
#if LLVM_ENABLE_THREADS
#include <thread>
namespace llvm {
-typedef std::thread thread;
+
+# if LLVM_ON_UNIX || _WIN32
+
+/// LLVM thread following std::thread interface with added constructor to
+/// specify stack size.
+class thread {
+ template<typename FPtr, typename ...Args, size_t ...Indices>
+ static void Apply(std::tuple<FPtr, Args...> &Callee, std::index_sequence<Indices...>) {
+ std::move(std::get<0>(Callee))(std::move(std::get<Indices + 1>(Callee))...);
+ }
+
+ template<typename CalleeTuple>
+ static void GenericThreadProxy(void *Ptr) {
+ std::unique_ptr<CalleeTuple> Callee(static_cast<CalleeTuple *>(Ptr));
+
+ // FIXME: use std::apply when C++17 is allowed.
+ std::make_index_sequence<std::tuple_size<CalleeTuple>() - 1> Indices{};
+ Apply(*Callee.get(), Indices);
+ }
+public:
+#if LLVM_ON_UNIX
+ using native_handle_type = pthread_t;
+ using id = pthread_t;
+ using start_routine_type = void *(*)(void *);
+
+ template <typename CalleeTuple> static void *ThreadProxy(void *Ptr) {
+ GenericThreadProxy<CalleeTuple>(Ptr);
+ return nullptr;
+ }
+#elif _WIN32
+ using native_handle_type = HANDLE;
+ using id = HANDLE;
+ using start_routine_type = unsigned (__stdcall *)(void *);
+
+ template <typename CalleeTuple> static unsigned __stdcall ThreadProxy(void *Ptr) {
+ GenericThreadProxy<CalleeTuple>(Ptr);
+ return 0;
+ }
+#endif
+
+#if defined(__APPLE__)
+ // Darwin's default stack size for threads except the main one is only 512KB,
+ // which is not enough for some/many normal LLVM compilations. This implements
+ // the same interface as std::thread but requests the same stack size as the
+ // main thread (8MB) before creation.
+ static const constexpr llvm::Optional<unsigned> DefaultStackSize =
+ 8 * 1024 * 1024;
+#else
+ static const constexpr llvm::Optional<unsigned> DefaultStackSize = None;
+#endif
+
+ thread() : Thread(native_handle_type()) {}
+ thread(thread &&Other) noexcept
+ : Thread(std::exchange(Other.Thread, native_handle_type())) {}
+
+ template <class Function, class... Args>
+ explicit thread(Function &&f, Args &&...args)
+ : thread(DefaultStackSize, f, args...) {}
+
+ template <class Function, class... Args>
+ explicit thread(llvm::Optional<unsigned> StackSizeInBytes, Function &&f,
+ Args &&...args);
+ thread(const thread &) = delete;
+
+ ~thread() {
+ if (joinable())
+ std::terminate();
+ }
+
+ thread &operator=(thread &&Other) noexcept {
+ if (joinable())
+ std::terminate();
+ Thread = std::exchange(Other.Thread, native_handle_type());
+ return *this;
+ }
+
+ bool joinable() const noexcept {
+ return Thread != native_handle_type();
+ }
+
+ id get_id() const noexcept {
+ return Thread;
+ }
+
+ native_handle_type native_handle() const noexcept {
+ return Thread;
+ }
+
+ static unsigned hardware_concurrency() {
+ return std::thread::hardware_concurrency();
+ };
+
+ inline void join();
+ inline void detach();
+
+ void swap(llvm::thread &Other) noexcept {
+ std::swap(Thread, Other.Thread);
+ }
+private:
+ native_handle_type Thread;
+};
+
+thread::native_handle_type
+llvm_execute_on_thread_impl(thread::start_routine_type ThreadFunc, void *Arg,
+ llvm::Optional<unsigned> StackSizeInBytes);
+void llvm_thread_join_impl(thread::native_handle_type Thread);
+void llvm_thread_detach_impl(thread::native_handle_type Thread);
+
+template <class Function, class... Args>
+thread::thread(llvm::Optional<unsigned> StackSizeInBytes, Function &&f,
+ Args &&...args) {
+ typedef std::tuple<typename std::decay<Function>::type,
+ typename std::decay<Args>::type...>
+ CalleeTuple;
+ std::unique_ptr<CalleeTuple> Callee(
+ new CalleeTuple(std::forward<Function>(f), std::forward<Args>(args)...));
+
+ Thread = llvm_execute_on_thread_impl(ThreadProxy<CalleeTuple>, Callee.get(),
+ StackSizeInBytes);
+ if (Thread != native_handle_type())
+ Callee.release();
+}
+
+void thread::join() {
+ llvm_thread_join_impl(Thread);
+ Thread = native_handle_type();
+}
+
+void thread::detach() {
+ llvm_thread_detach_impl(Thread);
+ Thread = native_handle_type();
}
+#else // !LLVM_ON_UNIX && !_WIN32
+
+/// std::thread backed implementation of llvm::thread interface that ignores the
+/// stack size request.
+class thread {
+public:
+ using native_handle_type = std::thread::native_handle_type;
+ using id = std::thread::id;
+
+ thread() : Thread(std::thread()) {}
+ thread(thread &&Other) noexcept
+ : Thread(std::exchange(Other.Thread, std::thread())) {}
+
+ template <class Function, class... Args>
+ explicit thread(llvm::Optional<unsigned> StackSizeInBytes, Function &&f,
+ Args &&...args)
+ : Thread(std::forward<Function>(f), std::forward<Args>(args)...) {}
+
+ template <class Function, class... Args>
+ explicit thread(Function &&f, Args &&...args) : Thread(f, args...) {}
+
+ thread(const thread &) = delete;
+
+ ~thread() {}
+
+ thread &operator=(thread &&Other) noexcept {
+ Thread = std::exchange(Other.Thread, std::thread());
+ return *this;
+ }
+
+ bool joinable() const noexcept {
+ return Thread.joinable();
+ }
+
+ id get_id() const noexcept {
+ return Thread.get_id();
+ }
+
+ native_handle_type native_handle() noexcept {
+ return Thread.native_handle();
+ }
+
+ static unsigned hardware_concurrency() {
+ return std::thread::hardware_concurrency();
+ };
+
+ inline void join() {
+ Thread.join();
+ }
+ inline void detach() {
+ Thread.detach();
+ }
+
+ void swap(llvm::thread &Other) noexcept {
+ std::swap(Thread, Other.Thread);
+ }
+private:
+ std::thread Thread;
+};
+
+#endif // LLVM_ON_UNIX || _WIN32
+
+} // namespace llvm
+
#else // !LLVM_ENABLE_THREADS
#include <utility>
@@ -36,11 +235,20 @@
thread() {}
thread(thread &&other) {}
template <class Function, class... Args>
+ explicit thread(llvm::Optional<unsigned> StackSizeInBytes, Function &&f,
+ Args &&...args) {
+ f(std::forward<Args>(args)...);
+ }
+ template <class Function, class... Args>
explicit thread(Function &&f, Args &&... args) {
f(std::forward<Args>(args)...);
}
thread(const thread &) = delete;
+ void detach() {
+ report_fatal_error("Detaching from a thread does not make sense with no "
+ "threading support");
+ }
void join() {}
static unsigned hardware_concurrency() { return 1; };
};
@@ -49,4 +257,4 @@
#endif // LLVM_ENABLE_THREADS
-#endif
+#endif // LLVM_SUPPORT_THREAD_H
\ No newline at end of file
Index: llvm/include/llvm/Support/Threading.h
===================================================================
--- llvm/include/llvm/Support/Threading.h
+++ llvm/include/llvm/Support/Threading.h
@@ -54,36 +54,6 @@
/// false otherwise.
bool llvm_is_multithreaded();
-/// Execute the given \p UserFn on a separate thread, passing it the provided \p
-/// UserData and waits for thread completion.
-///
-/// This function does not guarantee that the code will actually be executed
-/// on a separate thread or honoring the requested stack size, but tries to do
-/// so where system support is available.
-///
-/// \param UserFn - The callback to execute.
-/// \param UserData - An argument to pass to the callback function.
-/// \param StackSizeInBytes - A requested size (in bytes) for the thread stack
-/// (or None for default)
-void llvm_execute_on_thread(
- void (*UserFn)(void *), void *UserData,
- llvm::Optional<unsigned> StackSizeInBytes = llvm::None);
-
-/// Schedule the given \p Func for execution on a separate thread, then return
-/// to the caller immediately. Roughly equivalent to
-/// `std::thread(Func).detach()`, except it allows requesting a specific stack
-/// size, if supported for the platform.
-///
-/// This function would report a fatal error if it can't execute the code
-/// on a separate thread.
-///
-/// \param Func - The callback to execute.
-/// \param StackSizeInBytes - A requested size (in bytes) for the thread stack
-/// (or None for default)
-void llvm_execute_on_thread_async(
- llvm::unique_function<void()> Func,
- llvm::Optional<unsigned> StackSizeInBytes = llvm::None);
-
#if LLVM_THREADING_USE_STD_CALL_ONCE
typedef std::once_flag once_flag;
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===================================================================
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -87,7 +87,7 @@
/// a protected context which is run in another thread (optionally with a
/// requested stack size).
///
- /// See RunSafely() and llvm_execute_on_thread().
+ /// See RunSafely().
///
/// On Darwin, if PRIO_DARWIN_BG is set on the calling thread, it will be
/// propagated to the new thread as well.
Index: clang/tools/libclang/CIndex.cpp
===================================================================
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -52,6 +52,7 @@
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/thread.h"
#include "llvm/Support/Threading.h"
#include "llvm/Support/Timer.h"
#include "llvm/Support/raw_ostream.h"
@@ -6770,10 +6771,10 @@
void clang_executeOnThread(void (*fn)(void *), void *user_data,
unsigned stack_size) {
- llvm::llvm_execute_on_thread(fn, user_data,
- stack_size == 0
- ? clang::DesiredStackSize
- : llvm::Optional<unsigned>(stack_size));
+ llvm::thread Thread(stack_size == 0 ? clang::DesiredStackSize
+ : llvm::Optional<unsigned>(stack_size),
+ fn, user_data);
+ Thread.join();
}
//===----------------------------------------------------------------------===//
Index: clang-tools-extra/clangd/support/Threading.cpp
===================================================================
--- clang-tools-extra/clangd/support/Threading.cpp
+++ clang-tools-extra/clangd/support/Threading.cpp
@@ -2,6 +2,7 @@
#include "support/Trace.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/thread.h"
#include "llvm/Support/Threading.h"
#include <atomic>
#include <thread>
@@ -95,8 +96,10 @@
};
// Ensure our worker threads have big enough stacks to run clang.
- llvm::llvm_execute_on_thread_async(std::move(Task),
- /*clang::DesiredStackSize*/ 8 << 20);
+ llvm::thread Thread(
+ /*clang::DesiredStackSize*/ llvm::Optional<unsigned>(8 << 20),
+ std::move(Task));
+ Thread.detach();
}
Deadline timeoutSeconds(llvm::Optional<double> Seconds) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
