[llvm] 7bc7fe6 - Revert "[Support] Add a way to run a function on a detached thread"

Sam McCall via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 23 06:11:37 PDT 2019


Author: Sam McCall
Date: 2019-10-23T15:10:35+02:00
New Revision: 7bc7fe6b789d25d48d6dc71d533a411e9e981237

URL: https://github.com/llvm/llvm-project/commit/7bc7fe6b789d25d48d6dc71d533a411e9e981237
DIFF: https://github.com/llvm/llvm-project/commit/7bc7fe6b789d25d48d6dc71d533a411e9e981237.diff

LOG: Revert "[Support] Add a way to run a function on a detached thread"

This reverts commit 40668abca4d307e02b33345cfdb7271549ff48d0.
This causes clang tests to fail, as stacksize=0 is being explicitly passed and
is no longer a no-op.

Added: 
    

Modified: 
    llvm/include/llvm/Support/Threading.h
    llvm/lib/Support/Threading.cpp
    llvm/lib/Support/Unix/Threading.inc
    llvm/lib/Support/Unix/Unix.h
    llvm/lib/Support/Windows/Process.inc
    llvm/lib/Support/Windows/Threading.inc
    llvm/lib/Support/Windows/WindowsSupport.h
    llvm/unittests/Support/Threading.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/Threading.h b/llvm/include/llvm/Support/Threading.h
index bacab8fa23b6..46d413dc487b 100644
--- a/llvm/include/llvm/Support/Threading.h
+++ b/llvm/include/llvm/Support/Threading.h
@@ -14,7 +14,6 @@
 #ifndef LLVM_SUPPORT_THREADING_H
 #define LLVM_SUPPORT_THREADING_H
 
-#include "llvm/ADT/FunctionExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
 #include "llvm/Support/Compiler.h"
@@ -53,8 +52,9 @@ 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.
+/// llvm_execute_on_thread - 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
@@ -62,26 +62,10 @@ bool llvm_is_multithreaded();
 ///
 /// \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);
+/// \param RequestedStackSize - If non-zero, a requested size (in bytes) for
+/// the thread stack.
+void llvm_execute_on_thread(void (*UserFn)(void *), void *UserData,
+                            unsigned RequestedStackSize = 0);
 
 #if LLVM_THREADING_USE_STD_CALL_ONCE
 

diff  --git a/llvm/lib/Support/Threading.cpp b/llvm/lib/Support/Threading.cpp
index 48750cef5ec2..e5899a60f4db 100644
--- a/llvm/lib/Support/Threading.cpp
+++ b/llvm/lib/Support/Threading.cpp
@@ -12,7 +12,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Threading.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Host.h"
 
@@ -40,8 +39,8 @@ bool llvm::llvm_is_multithreaded() {
     (!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;
+                                  unsigned RequestedStackSize) {
+  (void)RequestedStackSize;
   Fn(UserData);
 }
 
@@ -57,25 +56,6 @@ void llvm::set_thread_name(const Twine &Name) {}
 
 void llvm::get_thread_name(SmallVectorImpl<char> &Name) { Name.clear(); }
 
-#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
 
 #include <thread>
@@ -104,17 +84,6 @@ unsigned llvm::hardware_concurrency() {
   return 1;
 }
 
-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"
@@ -123,20 +92,4 @@ enum class JoiningPolicy { Join, Detach };
 #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

diff  --git a/llvm/lib/Support/Unix/Threading.inc b/llvm/lib/Support/Unix/Threading.inc
index afb887fc1096..ed9a96563055 100644
--- a/llvm/lib/Support/Unix/Threading.inc
+++ b/llvm/lib/Support/Unix/Threading.inc
@@ -10,8 +10,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "Unix.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Twine.h"
 
@@ -42,56 +40,47 @@
 #include <unistd.h>      // For syscall()
 #endif
 
-static void *threadFuncSync(void *Arg) {
-  SyncThreadInfo *TI = static_cast<SyncThreadInfo *>(Arg);
-  TI->UserFn(TI->UserData);
-  return nullptr;
+namespace {
+  struct ThreadInfo {
+    void(*UserFn)(void *);
+    void *UserData;
+  };
 }
 
-static void *threadFuncAsync(void *Arg) {
-  std::unique_ptr<AsyncThreadInfo> Info(static_cast<AsyncThreadInfo *>(Arg));
-  (*Info)();
+static void *ExecuteOnThread_Dispatch(void *Arg) {
+  ThreadInfo *TI = reinterpret_cast<ThreadInfo*>(Arg);
+  TI->UserFn(TI->UserData);
   return nullptr;
 }
 
-static void
-llvm_execute_on_thread_impl(void *(*ThreadFunc)(void *), void *Arg,
-                            llvm::Optional<unsigned> StackSizeInBytes,
-                            JoiningPolicy JP) {
-  int errnum;
-
-  // Construct the attributes object.
+void llvm::llvm_execute_on_thread(void(*Fn)(void*), void *UserData,
+  unsigned RequestedStackSize) {
+  ThreadInfo Info = { Fn, UserData };
   pthread_attr_t Attr;
-  if ((errnum = ::pthread_attr_init(&Attr)) != 0) {
-    ReportErrnumFatal("pthread_attr_init failed", errnum);
-  }
+  pthread_t Thread;
 
-  auto AttrGuard = llvm::make_scope_exit([&] {
-    if ((errnum = ::pthread_attr_destroy(&Attr)) != 0) {
-      ReportErrnumFatal("pthread_attr_destroy failed", errnum);
-    }
-  });
+  // Construct the attributes object.
+  if (::pthread_attr_init(&Attr) != 0)
+    return;
 
   // Set the requested stack size, if given.
-  if (StackSizeInBytes) {
-    if ((errnum = ::pthread_attr_setstacksize(&Attr, *StackSizeInBytes)) != 0) {
-      ReportErrnumFatal("pthread_attr_setstacksize failed", errnum);
-    }
+  if (RequestedStackSize != 0) {
+    if (::pthread_attr_setstacksize(&Attr, RequestedStackSize) != 0)
+      goto error;
   }
 
   // Construct and execute the thread.
-  pthread_t Thread;
-  if ((errnum = ::pthread_create(&Thread, &Attr, ThreadFunc, Arg)) != 0)
-    ReportErrnumFatal("pthread_create failed", errnum);
+  if (::pthread_create(&Thread, &Attr, ExecuteOnThread_Dispatch, &Info) != 0)
+    goto error;
 
-  if (JP == JoiningPolicy::Join) {
-    // Wait for the thread
-    if ((errnum = ::pthread_join(Thread, nullptr)) != 0) {
-      ReportErrnumFatal("pthread_join failed", errnum);
-    }
-  }
+  // Wait for the thread and clean up.
+  ::pthread_join(Thread, nullptr);
+
+error:
+  ::pthread_attr_destroy(&Attr);
 }
 
+
 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/Unix/Unix.h b/llvm/lib/Support/Unix/Unix.h
index 1fc9a414f749..86309b0567f5 100644
--- a/llvm/lib/Support/Unix/Unix.h
+++ b/llvm/lib/Support/Unix/Unix.h
@@ -21,7 +21,6 @@
 #include "llvm/Config/config.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Errno.h"
-#include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 #include <assert.h>
 #include <cerrno>
@@ -70,14 +69,6 @@ static inline bool MakeErrMsg(
   return true;
 }
 
-// Include StrError(errnum) in a fatal error message.
-LLVM_ATTRIBUTE_NORETURN static inline void ReportErrnumFatal(const char *Msg,
-                                                             int errnum) {
-  std::string ErrMsg;
-  MakeErrMsg(&ErrMsg, Msg, errnum);
-  llvm::report_fatal_error(ErrMsg);
-}
-
 namespace llvm {
 namespace sys {
 

diff  --git a/llvm/lib/Support/Windows/Process.inc b/llvm/lib/Support/Windows/Process.inc
index 3526e3dee6fa..4b91f9f7fc66 100644
--- a/llvm/lib/Support/Windows/Process.inc
+++ b/llvm/lib/Support/Windows/Process.inc
@@ -439,6 +439,13 @@ const char *Process::ResetColor() {
   return 0;
 }
 
+// Include GetLastError() in a fatal error message.
+static void ReportLastErrorFatal(const char *Msg) {
+  std::string ErrMsg;
+  MakeErrMsg(&ErrMsg, Msg);
+  report_fatal_error(ErrMsg);
+}
+
 unsigned Process::GetRandomNumber() {
   HCRYPTPROV HCPC;
   if (!::CryptAcquireContextW(&HCPC, NULL, NULL, PROV_RSA_FULL,

diff  --git a/llvm/lib/Support/Windows/Threading.inc b/llvm/lib/Support/Windows/Threading.inc
index 83b47ea1e3df..96649472cc90 100644
--- a/llvm/lib/Support/Windows/Threading.inc
+++ b/llvm/lib/Support/Windows/Threading.inc
@@ -21,36 +21,36 @@
 #undef MemoryFence
 #endif
 
-static unsigned __stdcall threadFuncSync(void *Arg) {
-  SyncThreadInfo *TI = static_cast<SyncThreadInfo *>(Arg);
-  TI->UserFn(TI->UserData);
-  return 0;
+namespace {
+  struct ThreadInfo {
+    void(*func)(void*);
+    void *param;
+  };
 }
 
-static unsigned __stdcall threadFuncAsync(void *Arg) {
-  std::unique_ptr<AsyncThreadInfo> Info(static_cast<AsyncThreadInfo *>(Arg));
-  (*Info)();
+static unsigned __stdcall ThreadCallback(void *param) {
+  struct ThreadInfo *info = reinterpret_cast<struct ThreadInfo *>(param);
+  info->func(info->param);
+
   return 0;
 }
 
-static void
-llvm_execute_on_thread_impl(_beginthreadex_proc_type ThreadFunc, void *Arg,
-                            llvm::Optional<unsigned> StackSizeInBytes,
-                            JoiningPolicy JP) {
-  HANDLE hThread = (HANDLE)::_beginthreadex(
-      NULL, StackSizeInBytes.getValueOr(0), ThreadFunc, Arg, 0, NULL);
-
-  if (!hThread) {
-    ReportLastErrorFatal("_beginthreadex failed");
-  }
-
-  if (JP == JoiningPolicy::Join) {
-    if (::WaitForSingleObject(hThread, INFINITE) == WAIT_FAILED) {
-      ReportLastErrorFatal("WaitForSingleObject failed");
-    }
-  }
-  if (::CloseHandle(hThread) == FALSE) {
-    ReportLastErrorFatal("CloseHandle failed");
+void llvm::llvm_execute_on_thread(void(*Fn)(void*), void *UserData,
+  unsigned RequestedStackSize) {
+  struct ThreadInfo param = { Fn, UserData };
+
+  HANDLE hThread = (HANDLE)::_beginthreadex(NULL,
+    RequestedStackSize, ThreadCallback,
+    &param, 0, NULL);
+
+  if (hThread) {
+    // We actually don't care whether the wait succeeds or fails, in
+    // the same way we don't care whether the pthread_join call succeeds
+    // or fails.  There's not much we could do if this were to fail. But
+    // on success, this call will wait until the thread finishes executing
+    // before returning.
+    (void)::WaitForSingleObject(hThread, INFINITE);
+    ::CloseHandle(hThread);
   }
 }
 

diff  --git a/llvm/lib/Support/Windows/WindowsSupport.h b/llvm/lib/Support/Windows/WindowsSupport.h
index bb7e79b86018..2e2e97430b76 100644
--- a/llvm/lib/Support/Windows/WindowsSupport.h
+++ b/llvm/lib/Support/Windows/WindowsSupport.h
@@ -41,7 +41,6 @@
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Compiler.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/VersionTuple.h"
 #include <cassert>
 #include <string>
@@ -67,13 +66,6 @@ llvm::VersionTuple GetWindowsOSVersion();
 
 bool MakeErrMsg(std::string *ErrMsg, const std::string &prefix);
 
-// Include GetLastError() in a fatal error message.
-LLVM_ATTRIBUTE_NORETURN inline void ReportLastErrorFatal(const char *Msg) {
-  std::string ErrMsg;
-  MakeErrMsg(&ErrMsg, Msg);
-  llvm::report_fatal_error(ErrMsg);
-}
-
 template <typename HandleTraits>
 class ScopedHandle {
   typedef typename HandleTraits::handle_type handle_type;

diff  --git a/llvm/unittests/Support/Threading.cpp b/llvm/unittests/Support/Threading.cpp
index 21baa55ef443..01f1c5134482 100644
--- a/llvm/unittests/Support/Threading.cpp
+++ b/llvm/unittests/Support/Threading.cpp
@@ -10,9 +10,6 @@
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
-#include <atomic>
-#include <condition_variable>
-
 using namespace llvm;
 
 namespace {
@@ -24,55 +21,4 @@ TEST(Threading, PhysicalConcurrency) {
   ASSERT_LE(Num, thread::hardware_concurrency());
 }
 
-#if LLVM_ENABLE_THREADS
-
-class Notification {
-public:
-  void notify() {
-    {
-      std::lock_guard<std::mutex> Lock(M);
-      Notified = true;
-    }
-    CV.notify_all();
-  }
-
-  bool wait() {
-    std::unique_lock<std::mutex> Lock(M);
-    using steady_clock = std::chrono::steady_clock;
-    auto Deadline = steady_clock::now() +
-                    std::chrono::duration_cast<steady_clock::duration>(
-                        std::chrono::duration<double>(5));
-    return CV.wait_until(Lock, Deadline, [this] { return Notified; });
-  }
-
-private:
-  bool Notified = false;
-  mutable std::condition_variable CV;
-  mutable std::mutex M;
-};
-
-TEST(Threading, RunOnThreadSyncAsync) {
-  Notification ThreadStarted, ThreadAdvanced, ThreadFinished;
-
-  auto ThreadFunc = [&] {
-    ThreadStarted.notify();
-    ASSERT_TRUE(ThreadAdvanced.wait());
-    ThreadFinished.notify();
-  };
-
-  llvm::llvm_execute_on_thread_async(ThreadFunc);
-  ASSERT_TRUE(ThreadStarted.wait());
-  ThreadAdvanced.notify();
-  ASSERT_TRUE(ThreadFinished.wait());
-}
-
-TEST(Threading, RunOnThreadSync) {
-  std::atomic_bool Executed(false);
-  llvm::llvm_execute_on_thread(
-      [](void *Arg) { *static_cast<std::atomic_bool *>(Arg) = true; },
-      &Executed);
-  ASSERT_EQ(Executed, true);
-}
-#endif
-
 } // end anon namespace


        


More information about the llvm-commits mailing list