On Wed, Oct 16, 2013 at 12:04 PM, Nico Rieck <[email protected]> wrote:
> 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. > sched_yield() is part of the POSIX standard, same as pthreads. They were both part of the 'Threads option' in SUSv3 and both migrated to 'Base' in SUSv4. Also, please do not use __yield on Windows, it is apparently semantically equivalent to an x86 PAUSE instruction. A suitable replacement for sched_yield() on Windows would be SwitchToThread(). > * 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
