rmaprath added inline comments.
================
Comment at: include/__threading_support:201
@@ +200,3 @@
+// Mutex
+#define _LIBCPP_MUTEX_INITIALIZER nullptr
+struct __libcpp_platform_mutex_t;
----------------
bcraig wrote:
> rmaprath wrote:
> > bcraig wrote:
> > > rmaprath wrote:
> > > > bcraig wrote:
> > > > > I'm not sure I like taking the freedom to define
> > > > > _LIBCPP_MUTEX_INITIALIZER away from implementers.
> > > > >
> > > > > Would it be too terrible to replace this entire #elif block with
> > > > > something like the following?
> > > > >
> > > > > ```
> > > > > #if !defined(__has_include) || __has_include(<os_provided_thread.h>)
> > > > > #include <os_provided_thread.h>
> > > > > #else
> > > > > #error "_LIBCPP_THREAD_API_EXTERNAL requires the implementer to
> > > > > provide <os_provided_thread.h> in the include path"
> > > > > #endif
> > > > > ```
> > > > >
> > > > >
> > > > The problem is that, `std::mutex` constructor needs to be `constexpr`
> > > > (as you pointed out earlier). And since `__libcpp_mutex_t` is a pointer
> > > > type (for this externally threaded variant), sensible definitions for
> > > > `_LIBCPP_MUTEX_INITIALIZER` are limited.
> > > >
> > > > Other than `nullptr`, one may be able to define
> > > > `_LIBCPP_MUTEX_INITIALIZER` to be a pointer to some constant mutex
> > > > (presuming that make it `constexpr` OK?) but I'm not convinced if such
> > > > a setup would be very useful.
> > > >
> > > > Hope that sounds sensible?
> > > If the implementer gets to provide an entire header, then they also get
> > > to choose what libcpp_mutex_t will be. They could make it a struct.
> > This externally-threaded library variant needs to be compiled against a set
> > API, so we have this header with declarations like `__libcpp_mutex_lock()`
> > which are referred from within the library sources (same function
> > signatures as those used in `LIBCPP_THREAD_API_PTHREAD` - this allows us to
> > keep the library source changes to a minimum).
> >
> > Now, in this particular library variant, we want to differ these calls to
> > runtime rather than libcxx compile-time!
> >
> > On the other hand, I think you are proposing a compile-time (static) thread
> > porting setup where the platform vendors need to supply a header file with
> > appropriate definitions for these functions / types, and they would compile
> > libcxx themselves? That sounds like a good idea, but the current patch is
> > aiming for a different goal: we provide a pre-compiled libcxx library which
> > calls out to those `__libcpp_xxx` functions at runtime, and platform
> > vendors need to provide those functions, they don't have to compile libcxx
> > themselves. This is what forces us to use opaque pointer types to capture
> > platform-defined threading primitives.
> >
> > Perhaps I could improve the naming here, may be
> > `LIBCPP_HAS_RUNTIME_THREAD_API`? Or `LIBCPP_HAS_DYNAMIC_THREAD_API`?
> That is an excellent summary, and it does a good job of explaining the
> disconnect we were having. I'm going to push forward with the disconnect
> though :)
>
> I think you can implement the dynamic case in terms of the static case
> without loss of generality or performance. You ("Vendor X") could
> pre-compile your library with a pointer-sized libcpp_mutex_t defined in
> <os_provided_thread.h>, and at runtime call out to a different library that
> has an implementation of libcpp_mutex_lock. Someone else ("Vendor Y") could
> have a larger libcpp_mutex_t defined, and provide a static inline definition
> of libcpp_mutex_lock.
OK, I think I understand your point :)
Will meditate on it a bit and spin a new patch tomorrow. Thanks!
http://reviews.llvm.org/D20328
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
