[PATCH] D24864: [libcxxabi] Refactor pthread usage into a separate API

[Asiri Rathnayake via cfe-commits]

rmaprath added inline comments.

================
Comment at: src/config.h:22
@@ +21,3 @@
+
+#if defined(__GNUC__) || defined(__clang__)
+#define _LIBCXXABI_PRAGMA(_parameter_) _Pragma(#_parameter_)
----------------
EricWF wrote:
> What's the point of defining `_LIBCXXABI_WARNING`? It's unused and seems unneeded.
Good catch. I have copy-pasted this lot without checking.

================
Comment at: src/config.h:41
@@ +40,3 @@
+    defined(__CloudABI__) || \
+    defined(__sun__)
+#  define _LIBCXXABI_HAS_THREAD_API_PTHREAD
----------------
compnerd wrote:
> Can you change this to `defined(__sun__) && defined(_POSIX_THREADS)` at the very least?  There is no reason to assume pthreads on Solaris.  Solaris Threads are a valid threading model.
Not sure if I understand completely, `libcxxabi` currently either supports `pthread` or no-threads, either of which can be explicitly configured through a cmake option. So I don't see the point in checking for `_POSIX_THREADS` here as the only current threading option is `pthread`. When we add another threading system into the mix (like external threading), that too will be configurable with a cmake option, the defaulting to `pthread` is only when no particular threading system is selected.

Or is it that `pthread.h` et. al can be provided by Solaris Threads as well? So, the pthread API has two implementations on Solaris?

================
Comment at: src/config.h:42
@@ +41,3 @@
+    defined(__sun__)
+#  define _LIBCXXABI_HAS_THREAD_API_PTHREAD
+# else
----------------
compnerd wrote:
> Can we use `_LIBCXXABI_USE_THREAD_API_PTHREAD` instead?  You can have more than one threading API on a platform, and wish to use a specific one.
Makes sense. Need to change the convention used in `libcxx` too, but that can be done later.

================
Comment at: src/config.h:46
@@ +45,3 @@
+# endif
+#endif
+
----------------
compnerd wrote:
> I really think that we should use `_POSIX_THREADS` macro rather than this enumeration approach.
`_LIBCXXABI_USE_THREAD_API_XXXX` allows us to be consistent with naming of the different threading variants (e.g. I'm going to add `_LIBCXXABI_USE_THREAD_API_EXTENRAL` soon). Is there some functional benefit to using `_POSIX_THREADS` for the pthread variant?

================
Comment at: src/fallback_malloc.ipp:30
@@ -29,3 +29,3 @@
 #ifndef _LIBCXXABI_HAS_NO_THREADS
-static pthread_mutex_t heap_mutex = PTHREAD_MUTEX_INITIALIZER;
+static __libcxxabi_mutex_t heap_mutex = _LIBCXXABI_MUTEX_INITIALIZER;
 #else
----------------
Thanks for the catch.


https://reviews.llvm.org/D24864