[PATCH] D20874: [libcxx] Two more threading dependencies
rmaprath abandoned this revision. rmaprath added a comment. Most of this is no longer needed. I'll submit patches for the parts that are actually needed (e.g. nanosleep) later on. Thanks. https://reviews.llvm.org/D20874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20874: [libcxx] Two more threading dependencies
rmaprath updated this revision to Diff 59507.
rmaprath added a comment.
Added a comment about `~std::thread()` hook.
@EricWF: Gentle ping.
http://reviews.llvm.org/D20874
Files:
include/__threading_support
src/thread.cpp
Index: src/thread.cpp
===
--- src/thread.cpp
+++ src/thread.cpp
@@ -39,8 +39,10 @@
thread::~thread()
{
+// custom hook allowing thread implementations to control
+// the ~std::tread() behavior.
if (__t_ != 0)
-terminate();
+__libcpp_thread_terminate();
}
void
@@ -133,7 +135,7 @@
ts.tv_nsec = giga::num - 1;
}
-while (nanosleep(, ) == -1 && errno == EINTR)
+while (__libcpp_thread_nanosleep(, ) == -1 && errno == EINTR)
;
}
}
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -22,6 +22,8 @@
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
#include
#include
+#include
+#include
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -156,6 +158,12 @@
}
inline _LIBCPP_ALWAYS_INLINE
+int __libcpp_thread_nanosleep(const timespec *req, timespec *rem)
+{
+return nanosleep(req, rem);
+}
+
+inline _LIBCPP_ALWAYS_INLINE
int __libcpp_thread_join(__libcpp_thread_t* __t)
{
return pthread_join(*__t, 0);
@@ -173,6 +181,12 @@
sched_yield();
}
+inline _LIBCPP_ALWAYS_INLINE
+void __libcpp_thread_terminate()
+{
+std::terminate();
+}
+
// Thread local storage
typedef pthread_key_t __libcpp_tl_key;
Index: src/thread.cpp
===
--- src/thread.cpp
+++ src/thread.cpp
@@ -39,8 +39,10 @@
thread::~thread()
{
+// custom hook allowing thread implementations to control
+// the ~std::tread() behavior.
if (__t_ != 0)
-terminate();
+__libcpp_thread_terminate();
}
void
@@ -133,7 +135,7 @@
ts.tv_nsec = giga::num - 1;
}
-while (nanosleep(, ) == -1 && errno == EINTR)
+while (__libcpp_thread_nanosleep(, ) == -1 && errno == EINTR)
;
}
}
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -22,6 +22,8 @@
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
#include
#include
+#include
+#include
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -156,6 +158,12 @@
}
inline _LIBCPP_ALWAYS_INLINE
+int __libcpp_thread_nanosleep(const timespec *req, timespec *rem)
+{
+return nanosleep(req, rem);
+}
+
+inline _LIBCPP_ALWAYS_INLINE
int __libcpp_thread_join(__libcpp_thread_t* __t)
{
return pthread_join(*__t, 0);
@@ -173,6 +181,12 @@
sched_yield();
}
+inline _LIBCPP_ALWAYS_INLINE
+void __libcpp_thread_terminate()
+{
+std::terminate();
+}
+
// Thread local storage
typedef pthread_key_t __libcpp_tl_key;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20874: [libcxx] Two more threading dependencies
bcraig added inline comments.
Comment at: include/__threading_support:187
@@ +186,3 @@
+{
+std::terminate();
+}
rmaprath wrote:
> bcraig wrote:
> > Why does this need to go in threading_support? Seems like this is mostly
> > orthogonal to threading. libcxxabi seems like the better place to hold
> > changes to terminate anyway.
> I can clearly see the argument against it, was bit unsure of it myself.
>
> Now, the reason I want this the most is because of the externally threaded
> API, where I need to do some cleanup of the storage allocated in
> `__libcpp_thread_create()`. See the change I had to do in D20328 (thread.cpp)
> where I introduced a `__libcpp_thread_finalize()` function just for the
> cleanup. Thought it would be much cleaner to bundle up the two together so
> that I can avoid an explicit `#ifdef` in the sources.
>
> Is that enough of a justification? Or should I stick to the explicit `#ifdef`
> in the externally-threaded variant? I don't have a strong opinion here,
> either way is fine, this version is slightly more preferred.
I guess this is fine. I just need to tell myself (and maybe it should be
commented in the code?) that this isn't trying to replace std::terminate, it's
trying to replace a chunk of the std::thread dtor. Having a custom hook for
the std::thread dtor is reasonable, and this is a reasonable default
implementation.
Note that you only get a chance to do things here when a user does naughty
things and lets a joinable thread reach the std::thread dtor.
http://reviews.llvm.org/D20874
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20874: [libcxx] Two more threading dependencies
bcraig added inline comments.
Comment at: include/__threading_support:187
@@ +186,3 @@
+{
+std::terminate();
+}
Why does this need to go in threading_support? Seems like this is mostly
orthogonal to threading. libcxxabi seems like the better place to hold changes
to terminate anyway.
http://reviews.llvm.org/D20874
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20874: [libcxx] Two more threading dependencies
rmaprath created this revision.
rmaprath added reviewers: EricWF, bcraig, mclow.lists.
rmaprath added a subscriber: cfe-commits.
Bumped into these while working on the externally threaded variant. I think it
makes most sense to group these two under the main threading API. This greatly
simplifies the presentation of the externally threaded library variant as well.
http://reviews.llvm.org/D20874
Files:
include/__threading_support
src/thread.cpp
Index: src/thread.cpp
===
--- src/thread.cpp
+++ src/thread.cpp
@@ -40,7 +40,7 @@
thread::~thread()
{
if (__t_ != 0)
-terminate();
+__libcpp_thread_terminate();
}
void
@@ -130,7 +130,7 @@
ts.tv_nsec = giga::num - 1;
}
-while (nanosleep(, ) == -1 && errno == EINTR)
+while (__libcpp_thread_nanosleep(, ) == -1 && errno == EINTR)
;
}
}
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -22,6 +22,8 @@
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
#include
#include
+#include
+#include
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -156,6 +158,12 @@
}
inline _LIBCPP_ALWAYS_INLINE
+int __libcpp_thread_nanosleep(const timespec *req, timespec *rem)
+{
+return nanosleep(req, rem);
+}
+
+inline _LIBCPP_ALWAYS_INLINE
int __libcpp_thread_join(__libcpp_thread_t* __t)
{
return pthread_join(*__t, 0);
@@ -173,6 +181,12 @@
sched_yield();
}
+inline _LIBCPP_ALWAYS_INLINE
+void __libcpp_thread_terminate()
+{
+std::terminate();
+}
+
// Thread local storage
typedef pthread_key_t __libcpp_tl_key;
Index: src/thread.cpp
===
--- src/thread.cpp
+++ src/thread.cpp
@@ -40,7 +40,7 @@
thread::~thread()
{
if (__t_ != 0)
-terminate();
+__libcpp_thread_terminate();
}
void
@@ -130,7 +130,7 @@
ts.tv_nsec = giga::num - 1;
}
-while (nanosleep(, ) == -1 && errno == EINTR)
+while (__libcpp_thread_nanosleep(, ) == -1 && errno == EINTR)
;
}
}
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -22,6 +22,8 @@
#if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
#include
#include
+#include
+#include
#endif
_LIBCPP_BEGIN_NAMESPACE_STD
@@ -156,6 +158,12 @@
}
inline _LIBCPP_ALWAYS_INLINE
+int __libcpp_thread_nanosleep(const timespec *req, timespec *rem)
+{
+return nanosleep(req, rem);
+}
+
+inline _LIBCPP_ALWAYS_INLINE
int __libcpp_thread_join(__libcpp_thread_t* __t)
{
return pthread_join(*__t, 0);
@@ -173,6 +181,12 @@
sched_yield();
}
+inline _LIBCPP_ALWAYS_INLINE
+void __libcpp_thread_terminate()
+{
+std::terminate();
+}
+
// Thread local storage
typedef pthread_key_t __libcpp_tl_key;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
