On 13.10.2013 04:07, David Majnemer wrote:
Can we have a better name than 'futex' for these? Futex is a Linux kernel synchronization primitive and it's a bit confusing that you used the name here in this context.
Sure. I initially chose it because it's short and kind-of similar in that it also locks on an arbitrary pointer address.
On 15.10.2013 02:55, G M wrote:
* __mutex_base - I need the __yield function to be something like this for me to compile, otherwise I get compiler errors:
It's just a missing include. Technically sched_yield is not part of pthreads, even though Mingw implements it there. Since the implementation is the same there's no need to further special case it.
* General statement: Can we follow the pthread model more closely here for libcxx's Win32 path. Is there any good reason not to? i.e. with futex becoming pthread_mutex etc. etc.? I think pthreads is a pretty tight and well known interface and I think we should aim to have a default implementation in libcxx that is easily swappable for anyone else's implementation or even if it's not hot swappable so to speak. have the win32 model more closely follow it.
It's not intended as a pthreads port, and the features are far from complete for that. For one, there's no single mutex type for recursive and non-recursive mutexes. If someone is so inclined he can force usage of pthreads and use an external lib. Mimicking pthreads but only implementing a small part of it will only lead to problems.
* The _LIBCPP_PTHREADS macro name is a little confusing to me. I think it means, "use the existing path that say libcxx uses for pthreads today, otherwise using the new / built in one".
It just means "use pthreads". It's never used in negative checks, and the second path is always conditioned on _WIN32 being defined.
* The inline assembler might be able to be factored out, or out more: The win32 platform and clang appear to support the intrinsic bit test functions and see their are psdk intrinsics that provide the same thing.
Last I checked Clang doesn't support the MSVC bts intrinsic.
* Some thought / additional code might be required in cmakelists.txt to make all this work properly i.e. finding the right headers / libraries, making sure the additional required libraries get brought in for pthreads. I haven't got that far yet. On my machine I get missing symbols with this patch re. NtXXX functions. They aren't hard to resolve, but it would appear (if I'm correct) that by default these libraries aren't wired up for msvc etc yet.
Patch 2 adds ntdll.dll to the linker so not sure what's going wrong there.
* Phabricator. This patch might benefit from being discussed there. I haven't used it before nor have an account yet but I'll get one if that's a shared opinion. What do people think?
Too bad Phabricator doesn't support patch chains so you have to mash everything together. Multiple Phab entries that depend on each other are also a PITA.
Thanks for looking at this. -Nico _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
