This revision was automatically updated to reflect the committed changes.
Closed by commit rL281179: [libcxx] Introduce an externally-threaded libc++
variant. (authored by asiri).
Changed prior to commit:
https://reviews.llvm.org/D21968?vs=70840&id=70969#toc
Repository:
rL LLVM
https://revi
rmaprath updated this revision to Diff 70840.
rmaprath added a comment.
Herald added a subscriber: beanz.
Final patch incorporating all the changes from @EricWF and @compnerd.
Will commit tomorrow (@mclow.lists gave approval earlier)
/ Asiri
https://reviews.llvm.org/D21968
Files:
CMakeLists
rmaprath added a comment.
In https://reviews.llvm.org/D21968#534462, @EricWF wrote:
> This looks great. Two comments:
>
> 1. The declarations should be used in both the inline and external pthread
> implementation. They also need visibility declarations.
> 2. Why can't we use the inline impleme
mclow.lists added a comment.
The patch (again) doesn't apply cleanly (test/CMakeLists.txt), so I applied it
manually.
After that, it builds w/o errors, and passes all the tests.
After addressing @EricWF's concerns, I think this is ready to land.
https://reviews.llvm.org/D21968
_
EricWF added a comment.
This looks great. Two comments:
1. The declarations should be used in both the inline and external pthread
implementation. They also need visibility declarations.
2. Why can't we use the inline implementation to implement
`external_threads.cpp`?
I took a stab at it her
@mclow.lists: Ping ?
On 24 Aug 2016 22:46, "Asiri Rathnayake via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:
> rmaprath added inline comments.
>
>
> Comment at: include/__config:830
> @@ -829,1 +829,3 @@
> +!defined(_LIBCPP_HAS_THREAD_API_PTHREAD) && \
> +!defined(_L
rmaprath added inline comments.
Comment at: include/__config:830
@@ -829,1 +829,3 @@
+!defined(_LIBCPP_HAS_THREAD_API_PTHREAD) && \
+!defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
# if defined(__FreeBSD__) || \
compnerd wrote:
> I meant on the lines that you a
compnerd added inline comments.
Comment at: CMakeLists.txt:372
@@ +371,3 @@
+# Relax this restriction from HandleLLVMOptions
+string(REPLACE "-Wl,-z,defs" "" CMAKE_SHARED_LINKER_FLAGS
"${CMAKE_SHARED_LINKER_FLAGS}")
+ endif()
There is a similar change in
rmaprath updated this revision to Diff 69010.
rmaprath added a comment.
Addressed above comments from @mclow.lists.
Getting this to work on shared library builds was a bit tricky, had to use
`-undefined dynamic_lookup` linker option on OSX and strip off the default
`-Wl,-z,defs` linker option o
rmaprath added a comment.
Comments from @mclow.lists on the latest revision (received offline):
- Need instructions on how to build and test the patch...
- Need to be able to build+test on Mac (looks like there is a small problem in
the patch w.r.t pthreads that makes the build fail on Mac)
- Ne
rmaprath added inline comments.
Comment at: include/__external_threading:26
@@ +25,3 @@
+
+#if !defined(_LIBCPP_MUTEX_T) || \
+!defined(_LIBCPP_MUTEX_INITIALIZER) || \
mclow.lists wrote:
> bcraig wrote:
> > So users of external pthreading (or their compiler dr
mclow.lists added inline comments.
Comment at: include/__external_threading:26
@@ +25,3 @@
+
+#if !defined(_LIBCPP_MUTEX_T) || \
+!defined(_LIBCPP_MUTEX_INITIALIZER) || \
bcraig wrote:
> So users of external pthreading (or their compiler driver) would need to
rmaprath added a comment.
@compnerd: I plan to look at https://reviews.llvm.org/D18482 soon (going to
need it when this lands). Got moved to a new machine, still setting up the
environment :)
@mclow.lists: Ping?
https://reviews.llvm.org/D21968
__
rmaprath added inline comments.
Comment at: CMakeLists.txt:139
@@ -138,1 +138,3 @@
option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread
API" OFF)
+option(LIBCXX_HAS_EXTERNAL_THREAD_API
+ "Build libc++ with an externalized threading API.
compnerd added a subscriber: compnerd.
Comment at: CMakeLists.txt:139
@@ -138,1 +138,3 @@
option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread
API" OFF)
+option(LIBCXX_HAS_EXTERNAL_THREAD_API
+ "Build libc++ with an externalized threading API.
-
rmaprath updated this revision to Diff 68011.
rmaprath added a comment.
Rebased.
https://reviews.llvm.org/D21968
Files:
CMakeLists.txt
include/__config
include/__config_site.in
include/__threading_support
lib/CMakeLists.txt
test/CMakeLists.txt
test/libcxx/test/config.py
test/li
rmaprath updated this revision to Diff 63670.
rmaprath added a comment.
Minor cleanup + Ping.
http://reviews.llvm.org/D21968
Files:
CMakeLists.txt
include/__config
include/__config_site.in
include/__threading_support
lib/CMakeLists.txt
test/CMakeLists.txt
test/libcxx/test/config.p
rmaprath added a comment.
@mclow.lists, @ericwf: Gentle ping.
http://reviews.llvm.org/D21968
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
bcraig added a comment.
LGTM. As usual, you'll want to get one of the code owner's sign off first
though.
http://reviews.llvm.org/D21968
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com
rmaprath updated this revision to Diff 62847.
rmaprath added a comment.
Improve comment. NFC.
http://reviews.llvm.org/D21968
Files:
CMakeLists.txt
include/__config
include/__config_site.in
include/__threading_support
lib/CMakeLists.txt
test/CMakeLists.txt
test/libcxx/test/config.p
rmaprath marked 2 inline comments as done.
rmaprath added a comment.
http://reviews.llvm.org/D21968
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rmaprath updated this revision to Diff 62844.
rmaprath added a comment.
Addressed review comments from @bcraig:
- Got rid of some of the option name cleanups (done to make them more
consistent, but not relevant to the patch)
- Arranged it so that `libc++` vendors can drop in `__external_threadin
bcraig added inline comments.
Comment at: CMakeLists.txt:131
@@ -130,2 +130,3 @@
option(LIBCXX_HAS_MUSL_LIBC "Build libc++ with support for the Musl C library"
OFF)
-option(LIBCXX_HAS_PTHREAD_API "Ignore auto-detection and force use of pthread
API" OFF)
+option(LIBCXX_HAS_PTHRE
rmaprath created this revision.
rmaprath added reviewers: mclow.lists, EricWF, bcraig.
rmaprath added a subscriber: cfe-commits.
This is the replacement for D20328.
Rather than using opaque pointer types to delegate the internal representations
of mutex / condvar / thread types, in this patch we
24 matches
Mail list logo
