[libcxx-commits] [PATCH] D68480: Implementation of C++20's P1135R6 for libcxx

[Olivier Giroux via Phabricator via libcxx-commits]

__simt__ marked 6 inline comments as done.
__simt__ added inline comments.


================
Comment at: libcxx/include/atomic:1850
     typedef __atomic_base<_Tp> __base;
+    using value_type = _Tp;
     _LIBCPP_INLINE_VISIBILITY
----------------
ldionne wrote:
> Can we add a test for this?
Yes, I can do that.


================
Comment at: libcxx/include/atomic:1871
     typedef __atomic_base<_Tp*> __base;
+    using value_type = _Tp*;
     _LIBCPP_INLINE_VISIBILITY
----------------
ldionne wrote:
> This too!
Ack.


================
Comment at: libcxx/include/chrono:1571
 
+#ifndef _LIBCPP_HAS_CLOCK_API_EXTERNAL
+
----------------
ldionne wrote:
> What's that?
Timed wait functions need <chrono>. That meant that I had to port <chrono> to CUDA as part of this effort. In CUDA you can't ask the OS for the time, you need to ask the silicon for the time. I followed the precedent of external threading to implement this as external clocks. Makes sense?


================
Comment at: libcxx/include/semaphore:77
+
+__atomic_semaphore_base is the general-case implementation, to be used for
+user-requested least-max values that exceed the OS implementation support
----------------
ldionne wrote:
> Thanks for the comments in this file, I wish we did that more often.
NP, Eric had requested something here. Feel free to ask for comments elsewhere.


================
Comment at: libcxx/include/type_traits:4032
+#ifdef _LIBCPP_BEGIN_NAMESPACE_STD_NOVERSION
+_LIBCPP_BEGIN_NAMESPACE_STD_NOVERSION
+#else
----------------
ldionne wrote:
> What are these drive-by fixes?
It's important for the CUDA port that there not be naked "namespace std { ... }" that don't use the macros, because we need to inject our namespace name. I tried to keep the drive-by to a minimum.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68480/new/

https://reviews.llvm.org/D68480