espositofulvio added a comment.
In http://reviews.llvm.org/D11781#423520, @rmaprath wrote:
> @espositofulvio: Thanks for the patch! :)
>
> Committed as r268734.
Glad to see you land the patch! Great work :)
Repository:
rL LLVM
http://reviews.llvm.org/D11781
rmaprath added a comment.
@espositofulvio: Thanks for the patch! :)
Committed as r268734.
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
rmaprath added a comment.
In http://reviews.llvm.org/D11781#403349, @rmaprath wrote:
> In http://reviews.llvm.org/D11781#403343, @theraven wrote:
>
> > In http://reviews.llvm.org/D11781#403335, @rmaprath wrote:
> >
> > > For us (ARM), a threads porting layer is important on several RTOSes
> > >
rmaprath added a comment.
In http://reviews.llvm.org/D11781#403378, @espositofulvio wrote:
> In http://reviews.llvm.org/D11781#400968, @rmaprath wrote:
>
> > Hi, could I know the status of this? I'd like to push this forward.
> >
> > @espositofulvio: Are you working on this? (just checking since
espositofulvio added a comment.
In http://reviews.llvm.org/D11781#400968, @rmaprath wrote:
> Hi, could I know the status of this? I'd like to push this forward.
>
> @espositofulvio: Are you working on this? (just checking since this has gone
> stale for a while). @EricWF: I can create a
rmaprath added a comment.
In http://reviews.llvm.org/D11781#403343, @theraven wrote:
> In http://reviews.llvm.org/D11781#403335, @rmaprath wrote:
>
> > For us (ARM), a threads porting layer is important on several RTOSes (where
> > a full-blown pthreads implementations is not available). I will
theraven added a comment.
In http://reviews.llvm.org/D11781#403335, @rmaprath wrote:
> For us (ARM), a threads porting layer is important on several RTOSes (where a
> full-blown pthreads implementations is not available). I will see if I can
> publish one of those porting layer
rmaprath added a comment.
Hi Eric, Thanks for the comments!
I'll wait a bit for @espositofulvio in case if he wants to push this, otherwise
I'll create a new diff with the suggested changes.
For us (ARM), a threads porting layer is important on several RTOSes (where a
full-blown pthreads
EricWF added a comment.
I would like to see this patch without the `support/pthread/*.cpp` files. There
are a couple of reasons for this.
1. The symbols in those files are private to the dylib, but they are declared
in the headers and given external visibility.
2. Those symbols frequently use
rmaprath added a subscriber: rmaprath.
rmaprath added a comment.
Hi, could I know the status of this? I'd like to push this forward.
@espositofulvio: Are you working on this? (just checking since this has gone
stale for a while). @EricWF: I can create a separate diff for further review if
this
espositofulvio updated the summary for this revision.
espositofulvio updated this revision to Diff 34104.
espositofulvio added a comment.
- Addressed possible ABI breaks
- Reverted to not using a __config_file as @jroelofs has two separate patch for
that
Repository:
rL LLVM
EricWF added a comment.
Added more inline comments.
Comment at: include/__mutex_base:36
@@ -35,3 +37,3 @@
#else
- mutex() _NOEXCEPT {__m_ = (pthread_mutex_t)PTHREAD_MUTEX_INITIALIZER;}
#endif
Why was the cast insignificant?
Comment at:
EricWF added a reviewer: EricWF.
EricWF added a comment.
This patch has a long way to go but it has also come a long way. Here are a
couple of problems I see with it.
1. There are still plenty of ABI breaks. I'll try and point them all out.
2. This patch adds a lot of headers. libc++ has
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
jroelofs wrote:
espositofulvio wrote:
theraven wrote:
jroelofs added inline comments.
Comment at: include/__config:744
@@ +743,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# ifndef _LIBCPP_THREAD_API
+# error No thread API
The reason to use `CMAKE_BINARY_DIR` over `CMAKE_CURRENT_SOURCE_DIR` as the
location for this build
theraven added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
espositofulvio wrote:
jroelofs wrote:
espositofulvio wrote:
jroelofs wrote:
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
theraven wrote:
espositofulvio wrote:
jroelofs wrote:
espositofulvio wrote:
mclow.lists added a subscriber: mclow.lists.
mclow.lists added a reviewer: mclow.lists.
mclow.lists added a comment.
I agree with @jroelofs that we don't want to `#include unistd.h` in
`__config`
Repository:
rL LLVM
http://reviews.llvm.org/D11781
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
espositofulvio wrote:
theraven wrote:
espositofulvio wrote:
espositofulvio updated this revision to Diff 31716.
espositofulvio added a comment.
Added __CloudABI__ to the list of platform using pthread
Repository:
rL LLVM
http://reviews.llvm.org/D11781
Files:
include/__config
include/__mutex_base
include/mutex
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
@espositofulvio: @ed meant this:
```
#ifndef _WIN32
# include unistd.h
# if
ed added a comment.
A general note I have regarding this change:
Now that we're introducing separate implementations for mutexes and condition
variables, could we also consider letting `shared_mutex` and friends simply use
`pthread_rwlock_*()`? We currently have it implemented as a wrapper on
espositofulvio marked 5 inline comments as done.
espositofulvio added a comment.
Repository:
rL LLVM
http://reviews.llvm.org/D11781
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
jroelofs wrote:
@espositofulvio: @ed meant this:
```
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
espositofulvio wrote:
jroelofs wrote:
jroelofs wrote:
@espositofulvio: @ed meant
espositofulvio added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
espositofulvio wrote:
jroelofs wrote:
jroelofs wrote:
jroelofs added inline comments.
Comment at: include/__config:742
@@ +741,3 @@
+#ifndef _LIBCPP_HAS_NO_THREADS
+# if defined(__FreeBSD__) || \
+defined(__NetBSD__) || \
jroelofs wrote:
@espositofulvio: @ed meant this:
```
#ifndef _WIN32
# include
theraven added inline comments.
Comment at: include/__mutex_base:246
@@ -266,3 +245,3 @@
-class _LIBCPP_TYPE_VIS condition_variable
+class _LIBCPP_TYPE_VIS condition_variable : private
__libcxx_support::condition_variable
{
espositofulvio wrote:
theraven
28 matches
Mail list logo
