halyavin added inline comments.
================
Comment at: include/__threading_support:458
+ __libcpp_mutex_reference&& __m,
+ timespec* __ts)
+{
----------------
In posix, pthread_cond_timedwait uses absolute time but we use relative time
here.
================
Comment at: include/__threading_support:460
+{
+ using namespace _VSTD::chrono;
+ auto timeout = seconds(__ts->tv_sec) + nanoseconds(__ts->tv_nsec);
----------------
We need to include <chrono> to use types and functions from this namespace.
================
Comment at: include/__threading_support:464
+
+ _LIBCPP_ASSERT(timeout_ms.count() < _VSTD::numeric_limits<DWORD>::max(),
+ "timeout duration overflows");
----------------
You have done it accidentally right. I realized there is a second problem here
- if timeout equals to INFINITE, the thread will wait forever. INFINITE equals
to (DWORD)-1, so the strict sign in the assert is required. But for sanity sake
_VSTD::numeric_limits<DWORD>::max() should be replaced with INFINITE.
================
Comment at: include/__threading_support:486
+static inline _LIBCPP_ALWAYS_INLINE BOOL CALLBACK
+__libcpp_init_once_trampoline(PINIT_ONCE InitOnce, PVOID Parameter,
+ PVOID *Context)
----------------
We need underscores for parameters and init_routine.
================
Comment at: include/__threading_support:497
+
+int __libcpp_execute_once(__libcpp_exec_once_flag* flag,
+ void (*init_routine)(void))
----------------
Underscores for parameters are missing.
================
Comment at: include/__threading_support:527
+static inline _LIBCPP_ALWAYS_INLINE unsigned int WINAPI
+__libcpp_thread_trampoline(void *__data)
+{
----------------
Trampolines will never be inlined. Should we put them in support *.cpp instead?
The downside is new public symbols which can't be changed without breaking
backward compatibility. The upside is that we will have only one copy of each
trampoline. What do you think?
================
Comment at: include/__threading_support:531
+ *(__libcpp_thread_trampoline_data*)__data;
+ _VSTD::free(__data);
+ return reinterpret_cast<unsigned int>(data.__func(data.__arg));
----------------
Do we need #include <cstdlib> for free() and malloc() now? Can we use
new/delete instead?
BTW, What is the purpose of _VSTD? I think it is used to prevent
argument-dependent lookup and so is not needed for free and malloc.
================
Comment at: include/__threading_support:532
+ _VSTD::free(__data);
+ return reinterpret_cast<unsigned int>(data.__func(data.__arg));
+}
----------------
Should we even try to pass thread exit code, given that sizeof(unsigned int) <
sizeof(void*) on 64-bit system? std::thread doesn't support thread exit code
anyway.
Repository:
rL LLVM
https://reviews.llvm.org/D28220
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
