[libcxx][rfc] Externalized threading support

[rmaprath]

Summary

This patch builds on D19412.

The motivation here is to allow toolchain vendors to build a version of libcxx with all the underlying os threading mechanics differed to runtime - sacrificing some performance for flexibility.

This API (_LIBCPP_THREAD_API_EXTERNAL) currently only works when libcxx is build as a static library. This is because the shared library builds pass -z defs linker flag, which disallows undefined symbols in the library. I'm not familiar with shared library limitations on specific platforms, but in theory, this could work on shared library builds as well.

Currently there are quite a lot of test failures because the testing infrastructure need to be updated to provide an implementation of this threading API (pthreads will do). Our plan is to hide this whole extension under some cmake variable at some point.

Before putting more effort into this, I thought of checking if the community if open to this kind of an API. Of course, we (ARM) will be committed to maintaining this API in the long run.

[rmaprath]

I forgot to mention that we will need a similar (although much smaller) porting layer for libcxxabi and libunwind, as those too currently have a pthread dependency.

[bcraig]

Note: My opinion doesn't count for much as far as community acceptance goes.

I'm fine with the idea. I think there is at least one significant conformance hurdle that you will need to address though. I also have some commentary on the implementation details.

include/__mutex_base

43 ↗

(On Diff #54659)

This is not C++14 conformant. The default constructor for mutex() needs to be constexpr.

I'm not sure if there is a good way around this issue given the problem you are trying to solve. There have been similar constexpr issues with this constructor with regards to musl libc.

include/__os_support

23 ↗

(On Diff #54659)

I would prefer that the internal types use an under-under-libcpp __libcpp prefix. I've had issues in the past with my under-under symbol names colliding with those from the underlying C library, especially for obvious names like under-under-mutex_t.

src/algorithm.cpp

65

I would rather the new branch be used across the board. I don't like littering the generic implementation files with preprocessor blocks everywhere.

Stated differently, get rid of

__libcpp_os_support::__os_mutex __rs_mut

and just use

mutex __rs_mut

everywhere instead.

src/mutex.cpp

31

My preference is for all of this to hide behind the __os_mutex_destroy call, instead of having the preprocessor block here. It looks like that has already been discussed in a prior review though ( http://reviews.llvm.org/D11781 ).

[rmaprath]

Addressing review comments from @bcraig:

Adjusted according to the changes of D19412. This patch is now mostly trivial.

There is a small catch in that, this version of the API takes pointer-to-pointer type arguments for most __os_xxx functions. This is partly because we need to use opaque pointer types to hide the underlying implementation details from libcxx internals. This can be worked-around by using some template hackery (along the lines of is_pointer), but I thought to leave it as it is to avoid cluttering up the sources for the normal use case.

[bcraig]

LGTM. You will still need to get approval from one of your original reviewers though.

[rmaprath]

Updating to syc-up with D19412. Obviously, this patch needs more work in terms of setting up the testing story (using a pthreads based implementation of the threading API).

[rmaprath]

Re-sync with D19412.

[rmaprath]

Minor re-spin on D19412.

[rmaprath]

Re-spun on top of D19412.

[rmaprath]

Superseded by D20328.

include/__threading_support

 }
 
 inline _LIBCPP_ALWAYS_INLINE
 void __libcpp_tl_set(__libcpp_tl_key __key, void* __p)
 {
     pthread_setspecific(__key, __p);
 }
 
-#else // !_LIBCPP_THREAD_API_PTHREAD
+#elif defined(_LIBCPP_THREAD_API_EXTERNAL)
+
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER 0
+struct __libcpp_mutex_external;
+typedef __libcpp_mutex_external* __libcpp_mutex_t;
+
+int __libcpp_recursive_mutex_init(__libcpp_mutex_t* __m);
+
+int __libcpp_mutex_lock(__libcpp_mutex_t* __m);
+
+int __libcpp_mutex_trylock(__libcpp_mutex_t* __m);
+
+int __libcpp_mutex_unlock(__libcpp_mutex_t* __m);
+
+int __libcpp_mutex_destroy(__libcpp_mutex_t* __m);
+
+// Condition variable
+#define _LIBCPP_CONDVAR_INITIALIZER 0
+struct __libcpp_condvar_external;
+typedef __libcpp_condvar_external* __libcpp_condvar_t;
+
+int __libcpp_condvar_signal(__libcpp_condvar_t* __cv);
+
+int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv);
+
+int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m);
+
+int __libcpp_condvar_timedwait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m, timespec* __ts);
+
+int __libcpp_condvar_destroy(__libcpp_condvar_t* __cv);
+
+// Thread id
+typedef unsigned long __libcpp_thread_id;
+
+// Returns non-zero if the thread ids are equal, otherwise 0
+int __libcpp_thread_id_equal(__libcpp_thread_id t1, __libcpp_thread_id t2);
+
+// Returns non-zero if t1 < t2, otherwise 0
+int __libcpp_thread_id_less(__libcpp_thread_id t1, __libcpp_thread_id t2);
+
+// Thread
+struct __libcpp_thread_external;
+typedef __libcpp_thread_external* __libcpp_thread_t;
+
+int __libcpp_thread_create(__libcpp_thread_t* __t, void* (*__f)(void*), void* __arg);
+
+__libcpp_thread_id __libcpp_thread_get_current_id();
+
+__libcpp_thread_id __libcpp_thread_get_id(const __libcpp_thread_t* __t);
+
+int __libcpp_thread_join(__libcpp_thread_t* __t);
+
+int __libcpp_thread_detach(__libcpp_thread_t* __t);
+
+void __libcpp_thread_yield();
+
+// Thread local storage
+typedef unsigned long __libcpp_tl_key;
+
+int __libcpp_tl_create(__libcpp_tl_key* __key, void (*__at_exit) (void*));
+
+void* __libcpp_tl_get(__libcpp_tl_key __key);
+
+void __libcpp_tl_set(__libcpp_tl_key __key, void* __p);
+
+#else
   #error "No thread API selected."
 #endif
 
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_HAS_NO_THREADS
 
 #endif // _LIBCPP_THREADING_SUPPORT

src/algorithm.cpp

 template bool __insertion_sort_incomplete<__less<unsigned long long>&, unsigned long long*>(unsigned long long*, unsigned long long*, __less<unsigned long long>&);
 template bool __insertion_sort_incomplete<__less<float>&, float*>(float*, float*, __less<float>&);
 template bool __insertion_sort_incomplete<__less<double>&, double*>(double*, double*, __less<double>&);
 template bool __insertion_sort_incomplete<__less<long double>&, long double*>(long double*, long double*, __less<long double>&);
 
 template unsigned __sort5<__less<long double>&, long double*>(long double*, long double*, long double*, long double*, long double*, __less<long double>&);
 
 #ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(_LIBCPP_THREAD_API_EXTERNAL)
+static mutex __rs_mut;
+#  define __LOCK_LOCK() __rs_mut.lock();
+#  define __LOCK_UNLOCK() __rs_mut.unlock();
+# else
 static __libcpp_mutex_t __rs_mut = _LIBCPP_MUTEX_INITIALIZER;
+#  define __LOCK_LOCK() __libcpp_mutex_lock(&__rs_mut);
+#  define __LOCK_UNLOCK() __libcpp_mutex_unlock(&__rs_mut);
+# endif
 #endif
 unsigned __rs_default::__c_ = 0;
 
 __rs_default::__rs_default()
 {
 #ifndef _LIBCPP_HAS_NO_THREADS
-    __libcpp_mutex_lock(&__rs_mut);
+    __LOCK_LOCK()
 #endif
     __c_ = 1;
 }
 
 __rs_default::__rs_default(const __rs_default&)
 {
     ++__c_;
 }
 
 __rs_default::~__rs_default()
 {
 #ifndef _LIBCPP_HAS_NO_THREADS
     if (--__c_ == 0)
-       __libcpp_mutex_unlock(&__rs_mut);
+       __LOCK_UNLOCK()
 #else
     --__c_;
 #endif
 }
 
 __rs_default::result_type
 __rs_default::operator()()
 {
     static mt19937 __rs_g;
     return __rs_g();
 }
 
 __rs_default
 __rs_get()
 {
     return __rs_default();
 }
 
+#ifndef _LIBCPP_HAS_NO_THREADS
+# undef __LOCK_LOCK
+# undef __LOCK_UNLOCK
+#endif
+
 _LIBCPP_END_NAMESPACE_STD

src/memory.cpp

     return 0;
 }
 
 #endif  // _LIBCPP_NO_RTTI
 
 #if defined(_LIBCPP_HAS_C_ATOMIC_IMP) && !defined(_LIBCPP_HAS_NO_THREADS)
 
 static const std::size_t __sp_mut_count = 16;
+#if defined(_LIBCPP_THREAD_API_EXTERNAL)
+static mutex mut_back_imp[__sp_mut_count];
+#else
 static __libcpp_mutex_t mut_back_imp[__sp_mut_count] =
 {
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER,
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER,
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER,
     _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER, _LIBCPP_MUTEX_INITIALIZER
 };
+#endif
 
 static mutex* mut_back = reinterpret_cast<std::mutex*>(mut_back_imp);
 
 _LIBCPP_CONSTEXPR __sp_mut::__sp_mut(void* p) _NOEXCEPT
    : __lx(p)
 {
 }
 

src/mutex.cpp

 
 // If dispatch_once_f ever handles C++ exceptions, and if one can get to it
 // without illegal macros (unexpected macros not beginning with _UpperCase or
 // __lowercase), and if it stops spinning waiting threads, then call_once should
 // call into dispatch_once_f instead of here. Relevant radar this code needs to
 // keep in sync with:  7741191.
 
 #ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(_LIBCPP_THREAD_API_EXTERNAL)
+static mutex mut;
+static condition_variable cv;
+#  define __LOCK_INIT() unique_lock<mutex> lk(mut);
+#  define __LOCK_LOCK() lk.lock();
+#  define __LOCK_UNLOCK() lk.unlock();
+#  define __CV_WAIT_LOCK() cv.wait(lk);
+#  define __CV_NOTIFY_ALL() cv.notify_all();
+# else
 static __libcpp_mutex_t mut = _LIBCPP_MUTEX_INITIALIZER;
 static __libcpp_condvar_t cv = _LIBCPP_CONDVAR_INITIALIZER;
+#  define __LOCK_INIT() __libcpp_mutex_lock(&mut);
+#  define __LOCK_LOCK() __libcpp_mutex_lock(&mut);
+#  define __LOCK_UNLOCK() __libcpp_mutex_unlock(&mut);
+#  define __CV_WAIT_LOCK() __libcpp_condvar_wait(&cv, &mut);
+#  define __CV_NOTIFY_ALL() __libcpp_condvar_broadcast(&cv);
+# endif
 #endif
 
 /// NOTE: Changes to flag are done via relaxed atomic stores
 ///       even though the accesses are protected by a mutex because threads
 ///       just entering 'call_once` concurrently read from flag.
 void
 __call_once(volatile unsigned long& flag, void* arg, void(*func)(void*))
 {

         catch (...)
         {
             flag = 0ul;
             throw;
         }
 #endif  // _LIBCPP_NO_EXCEPTIONS
     }
 #else // !_LIBCPP_HAS_NO_THREADS
-    __libcpp_mutex_lock(&mut);
+    __LOCK_INIT()
     while (flag == 1)
-        __libcpp_condvar_wait(&cv, &mut);
+        __CV_WAIT_LOCK()
     if (flag == 0)
     {
 #ifndef _LIBCPP_NO_EXCEPTIONS
         try
         {
 #endif  // _LIBCPP_NO_EXCEPTIONS
             __libcpp_relaxed_store(&flag, 1ul);
-            __libcpp_mutex_unlock(&mut);
+            __LOCK_UNLOCK()
             func(arg);
-            __libcpp_mutex_lock(&mut);
+            __LOCK_LOCK()
             __libcpp_relaxed_store(&flag, ~0ul);
-            __libcpp_mutex_unlock(&mut);
-            __libcpp_condvar_broadcast(&cv);
+            __LOCK_UNLOCK()
+            __CV_NOTIFY_ALL()
 #ifndef _LIBCPP_NO_EXCEPTIONS
         }
         catch (...)
         {
-            __libcpp_mutex_lock(&mut);
+            __LOCK_LOCK()
             __libcpp_relaxed_store(&flag, 0ul);
-            __libcpp_mutex_unlock(&mut);
-            __libcpp_condvar_broadcast(&cv);
+            __LOCK_UNLOCK()
+            __CV_NOTIFY_ALL()
             throw;
         }
 #endif  // _LIBCPP_NO_EXCEPTIONS
     }
     else
-        __libcpp_mutex_unlock(&mut);
+        __LOCK_UNLOCK()
 #endif // !_LIBCPP_HAS_NO_THREADS
 
 }
 
+#ifndef _LIBCPP_HAS_NO_THREADS
+# undef __LOCK_INIT
+# undef __LOCK_LOCK
+# undef __LOCK_UNLOCK
+# undef __CV_WAIT_LOCK
+# undef __CV_NOTIFY_ALL
+#endif
+
 _LIBCPP_END_NAMESPACE_STD