From 2bf5e8d953ededbc208bd4c116c9d6331d73f0f0 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Thu, 8 Jul 2021 14:59:47 +0100 Subject: [PATCH] Revert "Support: add llvm::thread class that supports specifying stack size." It's causing build failures because DefaultStackSize isn't defined everywhere it should be and I need time to investigate. --- clang-tools-extra/clangd/support/Threading.cpp | 7 +- clang/tools/libclang/CIndex.cpp | 9 +- llvm/include/llvm/Support/CrashRecoveryContext.h | 2 +- llvm/include/llvm/Support/Threading.h | 30 ++++ llvm/include/llvm/Support/thread.h | 216 +---------------------- llvm/lib/Support/CrashRecoveryContext.cpp | 11 +- llvm/lib/Support/ThreadPool.cpp | 4 +- llvm/lib/Support/Threading.cpp | 54 +++++- llvm/lib/Support/Unix/Threading.inc | 52 +++--- llvm/lib/Support/Windows/Threading.inc | 41 +++-- llvm/unittests/Support/Threading.cpp | 17 +- 11 files changed, 147 insertions(+), 296 deletions(-) diff --git a/clang-tools-extra/clangd/support/Threading.cpp b/clang-tools-extra/clangd/support/Threading.cpp index 4d50776..7f3bd62 100644 --- a/clang-tools-extra/clangd/support/Threading.cpp +++ b/clang-tools-extra/clangd/support/Threading.cpp @@ -3,7 +3,6 @@ #include "llvm/ADT/ScopeExit.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Threading.h" -#include "llvm/Support/thread.h" #include #include #ifdef __USE_POSIX @@ -96,10 +95,8 @@ void AsyncTaskRunner::runAsync(const llvm::Twine &Name, }; // Ensure our worker threads have big enough stacks to run clang. - llvm::thread Thread( - /*clang::DesiredStackSize*/ llvm::Optional(8 << 20), - std::move(Task)); - Thread.detach(); + llvm::llvm_execute_on_thread_async(std::move(Task), + /*clang::DesiredStackSize*/ 8 << 20); } Deadline timeoutSeconds(llvm::Optional Seconds) { diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index 72eae1e..e3d34e1 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -55,7 +55,6 @@ #include "llvm/Support/Threading.h" #include "llvm/Support/Timer.h" #include "llvm/Support/raw_ostream.h" -#include "llvm/Support/thread.h" #include #if LLVM_ENABLE_THREADS != 0 && defined(__APPLE__) @@ -6786,10 +6785,10 @@ void clang_enableStackTraces(void) { void clang_executeOnThread(void (*fn)(void *), void *user_data, unsigned stack_size) { - llvm::thread Thread(stack_size == 0 ? clang::DesiredStackSize - : llvm::Optional(stack_size), - fn, user_data); - Thread.join(); + llvm::llvm_execute_on_thread(fn, user_data, + stack_size == 0 + ? clang::DesiredStackSize + : llvm::Optional(stack_size)); } //===----------------------------------------------------------------------===// diff --git a/llvm/include/llvm/Support/CrashRecoveryContext.h b/llvm/include/llvm/Support/CrashRecoveryContext.h index 4986906..f756635 100644 --- a/llvm/include/llvm/Support/CrashRecoveryContext.h +++ b/llvm/include/llvm/Support/CrashRecoveryContext.h @@ -87,7 +87,7 @@ public: /// a protected context which is run in another thread (optionally with a /// requested stack size). /// - /// See RunSafely(). + /// See RunSafely() and llvm_execute_on_thread(). /// /// On Darwin, if PRIO_DARWIN_BG is set on the calling thread, it will be /// propagated to the new thread as well. diff --git a/llvm/include/llvm/Support/Threading.h b/llvm/include/llvm/Support/Threading.h index 94de950..e489caf4 100644 --- a/llvm/include/llvm/Support/Threading.h +++ b/llvm/include/llvm/Support/Threading.h @@ -55,6 +55,36 @@ class Twine; /// 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 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 Func, + llvm::Optional StackSizeInBytes = llvm::None); + #if LLVM_THREADING_USE_STD_CALL_ONCE typedef std::once_flag once_flag; diff --git a/llvm/include/llvm/Support/thread.h b/llvm/include/llvm/Support/thread.h index 93b8370..084ed16 100644 --- a/llvm/include/llvm/Support/thread.h +++ b/llvm/include/llvm/Support/thread.h @@ -16,215 +16,16 @@ #ifndef LLVM_SUPPORT_THREAD_H #define LLVM_SUPPORT_THREAD_H -#include "llvm/ADT/Optional.h" #include "llvm/Config/llvm-config.h" -#ifdef _WIN32 -typedef unsigned long DWORD; -typedef void *PVOID; -typedef PVOID HANDLE; -#endif - #if LLVM_ENABLE_THREADS #include namespace llvm { - -#if LLVM_ON_UNIX || _WIN32 - -/// LLVM thread following std::thread interface with added constructor to -/// specify stack size. -class thread { - template - static void Apply(std::tuple &Callee, - std::index_sequence) { - std::move(std::get<0>(Callee))(std::move(std::get(Callee))...); - } - - template static void GenericThreadProxy(void *Ptr) { - std::unique_ptr Callee(static_cast(Ptr)); - - // FIXME: use std::apply when C++17 is allowed. - std::make_index_sequence() - 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 static void *ThreadProxy(void *Ptr) { - GenericThreadProxy(Ptr); - return nullptr; - } -#elif _WIN32 - using native_handle_type = HANDLE; - using id = DWORD; - using start_routine_type = unsigned(__stdcall *)(void *); - - template - static unsigned __stdcall ThreadProxy(void *Ptr) { - GenericThreadProxy(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 DefaultStackSize = - 8 * 1024 * 1024; -#else - static const constexpr llvm::Optional DefaultStackSize = None; -#endif - - thread() : Thread(native_handle_type()) {} - thread(thread &&Other) noexcept - : Thread(std::exchange(Other.Thread, native_handle_type())) {} - - template - explicit thread(Function &&f, Args &&...args) - : thread(DefaultStackSize, f, args...) {} - - template - explicit thread(llvm::Optional 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(); } - - inline id get_id() const noexcept; - - 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 StackSizeInBytes); -void llvm_thread_join_impl(thread::native_handle_type Thread); -void llvm_thread_detach_impl(thread::native_handle_type Thread); -thread::id llvm_thread_get_id_impl(thread::native_handle_type Thread); -thread::id llvm_thread_get_current_id_impl(); - -template -thread::thread(llvm::Optional StackSizeInBytes, Function &&f, - Args &&...args) { - typedef std::tuple::type, - typename std::decay::type...> - CalleeTuple; - std::unique_ptr Callee( - new CalleeTuple(std::forward(f), std::forward(args)...)); - - Thread = llvm_execute_on_thread_impl(ThreadProxy, Callee.get(), - StackSizeInBytes); - if (Thread != native_handle_type()) - Callee.release(); +typedef std::thread thread; } -thread::id thread::get_id() const noexcept { - return llvm_thread_get_id_impl(Thread); -} - -void thread::join() { - llvm_thread_join_impl(Thread); - Thread = native_handle_type(); -} - -void thread::detach() { - llvm_thread_detach_impl(Thread); - Thread = native_handle_type(); -} - -namespace this_thread { -inline thread::id get_id() { return llvm_thread_get_current_id_impl(); } -} // namespace this_thread - -#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 - explicit thread(llvm::Optional StackSizeInBytes, Function &&f, - Args &&...args) - : Thread(std::forward(f), std::forward(args)...) {} - - template - 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; -}; - -namespace this_thread { - inline thread::id get_id() { return std::this_thread::get_id(); } -} - -#endif // LLVM_ON_UNIX || _WIN32 - -} // namespace llvm - #else // !LLVM_ENABLE_THREADS #include @@ -235,26 +36,17 @@ struct thread { thread() {} thread(thread &&other) {} template - explicit thread(llvm::Optional StackSizeInBytes, Function &&f, - Args &&...args) { - f(std::forward(args)...); - } - template - explicit thread(Function &&f, Args &&...args) { + explicit thread(Function &&f, Args &&... args) { f(std::forward(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; }; }; -} // namespace llvm +} #endif // LLVM_ENABLE_THREADS -#endif // LLVM_SUPPORT_THREAD_H +#endif diff --git a/llvm/lib/Support/CrashRecoveryContext.cpp b/llvm/lib/Support/CrashRecoveryContext.cpp index 433d99d..3ee6a4f 100644 --- a/llvm/lib/Support/CrashRecoveryContext.cpp +++ b/llvm/lib/Support/CrashRecoveryContext.cpp @@ -13,7 +13,6 @@ #include "llvm/Support/ManagedStatic.h" #include "llvm/Support/Signals.h" #include "llvm/Support/ThreadLocal.h" -#include "llvm/Support/thread.h" #include #include @@ -501,12 +500,10 @@ bool CrashRecoveryContext::RunSafelyOnThread(function_ref Fn, unsigned RequestedStackSize) { bool UseBackgroundPriority = hasThreadBackgroundPriority(); RunSafelyOnThreadInfo Info = { Fn, this, UseBackgroundPriority, false }; - llvm::thread Thread(RequestedStackSize == 0 - ? llvm::None - : llvm::Optional(RequestedStackSize), - RunSafelyOnThread_Dispatch, &Info); - Thread.join(); - + llvm_execute_on_thread(RunSafelyOnThread_Dispatch, &Info, + RequestedStackSize == 0 + ? llvm::None + : llvm::Optional(RequestedStackSize)); if (CrashRecoveryContextImpl *CRC = (CrashRecoveryContextImpl *)Impl) CRC->setSwitchedThread(); return Info.Result; diff --git a/llvm/lib/Support/ThreadPool.cpp b/llvm/lib/Support/ThreadPool.cpp index 81926d8..f442b3b 100644 --- a/llvm/lib/Support/ThreadPool.cpp +++ b/llvm/lib/Support/ThreadPool.cpp @@ -73,8 +73,8 @@ void ThreadPool::wait() { } bool ThreadPool::isWorkerThread() const { - llvm::thread::id CurrentThreadId = llvm::this_thread::get_id(); - for (const llvm::thread &Thread : Threads) + std::thread::id CurrentThreadId = std::this_thread::get_id(); + for (const std::thread &Thread : Threads) if (CurrentThreadId == Thread.get_id()) return true; return false; diff --git a/llvm/lib/Support/Threading.cpp b/llvm/lib/Support/Threading.cpp index 55e97d6..61f8ee5 100644 --- a/llvm/lib/Support/Threading.cpp +++ b/llvm/lib/Support/Threading.cpp @@ -15,7 +15,6 @@ #include "llvm/ADT/Optional.h" #include "llvm/Config/config.h" #include "llvm/Support/Host.h" -#include "llvm/Support/thread.h" #include #include @@ -39,6 +38,13 @@ bool llvm::llvm_is_multithreaded() { #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 StackSizeInBytes) { + (void)StackSizeInBytes; + Fn(UserData); +} + uint64_t llvm::get_threadid() { return 0; } uint32_t llvm::get_max_thread_name_length() { return 0; } @@ -54,6 +60,25 @@ unsigned llvm::ThreadPoolStrategy::compute_thread_count() const { return 1; } +#if LLVM_ENABLE_THREADS == 0 +void llvm::llvm_execute_on_thread_async( + llvm::unique_function Func, + llvm::Optional 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 Func, + llvm::Optional StackSizeInBytes) { + (void)StackSizeInBytes; + std::thread(std::move(Func)).detach(); +} +#endif + #else int computeHostNumHardwareThreads(); @@ -70,6 +95,17 @@ unsigned llvm::ThreadPoolStrategy::compute_thread_count() const { return std::min((unsigned)MaxThreadCount, ThreadsRequested); } +namespace { +struct SyncThreadInfo { + void (*UserFn)(void *); + void *UserData; +}; + +using AsyncThreadInfo = llvm::unique_function; + +enum class JoiningPolicy { Join, Detach }; +} // namespace + // Include the platform-specific parts of this class. #ifdef LLVM_ON_UNIX #include "Unix/Threading.inc" @@ -78,6 +114,22 @@ unsigned llvm::ThreadPoolStrategy::compute_thread_count() const { #include "Windows/Threading.inc" #endif +void llvm::llvm_execute_on_thread(void (*Fn)(void *), void *UserData, + llvm::Optional 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 Func, + llvm::Optional StackSizeInBytes) { + llvm_execute_on_thread_impl(&threadFuncAsync, + new AsyncThreadInfo(std::move(Func)), + StackSizeInBytes, JoiningPolicy::Detach); +} + #endif Optional diff --git a/llvm/lib/Support/Unix/Threading.inc b/llvm/lib/Support/Unix/Threading.inc index 2131def..667d023 100644 --- a/llvm/lib/Support/Unix/Threading.inc +++ b/llvm/lib/Support/Unix/Threading.inc @@ -48,9 +48,22 @@ #include // For syscall() #endif -pthread_t -llvm::llvm_execute_on_thread_impl(void *(*ThreadFunc)(void *), void *Arg, - llvm::Optional StackSizeInBytes) { +static void *threadFuncSync(void *Arg) { + SyncThreadInfo *TI = static_cast(Arg); + TI->UserFn(TI->UserData); + return nullptr; +} + +static void *threadFuncAsync(void *Arg) { + std::unique_ptr Info(static_cast(Arg)); + (*Info)(); + return nullptr; +} + +static void +llvm_execute_on_thread_impl(void *(*ThreadFunc)(void *), void *Arg, + llvm::Optional StackSizeInBytes, + JoiningPolicy JP) { int errnum; // Construct the attributes object. @@ -77,33 +90,18 @@ llvm::llvm_execute_on_thread_impl(void *(*ThreadFunc)(void *), void *Arg, if ((errnum = ::pthread_create(&Thread, &Attr, ThreadFunc, Arg)) != 0) ReportErrnumFatal("pthread_create 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); + 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); + } } } -pthread_t llvm::llvm_thread_get_id_impl(pthread_t Thread) { - return Thread; -} - -pthread_t llvm::llvm_thread_get_current_id_impl() { - return ::pthread_self(); -} - uint64_t llvm::get_threadid() { #if defined(__APPLE__) // Calling "mach_thread_self()" bumps the reference count on the thread diff --git a/llvm/lib/Support/Windows/Threading.inc b/llvm/lib/Support/Windows/Threading.inc index 12d8cbc..6448bb4 100644 --- a/llvm/lib/Support/Windows/Threading.inc +++ b/llvm/lib/Support/Windows/Threading.inc @@ -23,10 +23,22 @@ #undef MemoryFence #endif -HANDLE -llvm::llvm_execute_on_thread_impl(unsigned(__stdcall *ThreadFunc)(void *), - void *Arg, - llvm::Optional StackSizeInBytes) { +static unsigned __stdcall threadFuncSync(void *Arg) { + SyncThreadInfo *TI = static_cast(Arg); + TI->UserFn(TI->UserData); + return 0; +} + +static unsigned __stdcall threadFuncAsync(void *Arg) { + std::unique_ptr Info(static_cast(Arg)); + (*Info)(); + return 0; +} + +static void +llvm_execute_on_thread_impl(unsigned (__stdcall *ThreadFunc)(void *), void *Arg, + llvm::Optional StackSizeInBytes, + JoiningPolicy JP) { HANDLE hThread = (HANDLE)::_beginthreadex( NULL, StackSizeInBytes.getValueOr(0), ThreadFunc, Arg, 0, NULL); @@ -34,29 +46,16 @@ llvm::llvm_execute_on_thread_impl(unsigned(__stdcall *ThreadFunc)(void *), ReportLastErrorFatal("_beginthreadex failed"); } - return hThread; -} - -void llvm::llvm_thread_join_impl(HANDLE hThread) { - if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) { - ReportLastErrorFatal("WaitForSingleObject failed"); + if (JP == JoiningPolicy::Join) { + if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) { + ReportLastErrorFatal("WaitForSingleObject failed"); + } } -} - -void llvm::llvm_thread_detach_impl(HANDLE hThread) { if (::CloseHandle(hThread) == FALSE) { ReportLastErrorFatal("CloseHandle failed"); } } -DWORD llvm::llvm_thread_get_id_impl(HANDLE hThread) { - return ::GetThreadId(hThread); -} - -DWORD llvm::llvm_thread_get_current_id_impl() { - return ::GetCurrentThreadId(); -} - uint64_t llvm::get_threadid() { return uint64_t(::GetCurrentThreadId()); } diff --git a/llvm/unittests/Support/Threading.cpp b/llvm/unittests/Support/Threading.cpp index 69a2987..c76e6e4 100644 --- a/llvm/unittests/Support/Threading.cpp +++ b/llvm/unittests/Support/Threading.cpp @@ -63,8 +63,7 @@ TEST(Threading, RunOnThreadSyncAsync) { ThreadFinished.notify(); }; - llvm::thread Thread(ThreadFunc); - Thread.detach(); + llvm::llvm_execute_on_thread_async(ThreadFunc); ASSERT_TRUE(ThreadStarted.wait()); ThreadAdvanced.notify(); ASSERT_TRUE(ThreadFinished.wait()); @@ -72,23 +71,11 @@ TEST(Threading, RunOnThreadSyncAsync) { TEST(Threading, RunOnThreadSync) { std::atomic_bool Executed(false); - llvm::thread Thread( + llvm::llvm_execute_on_thread( [](void *Arg) { *static_cast(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 -- 2.7.4