[PATCH] [libcxx] Windows port of thread/mutex/chrono

Nico Rieck nico.rieck at gmail.com
Wed Oct 16 12:04:04 PDT 2013


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




More information about the cfe-commits mailing list