[LLVMdev] [PATCH] Replace the Execution Engine's mutex with std::recursive_mutex

Zachary Turner zturner at google.com
Fri Jun 20 13:51:43 PDT 2014 

Is this only the mutual exclusion classes  (std::*mutex) or is this also
other functionality like call_once, lock_guard, etc?


On Fri, Jun 20, 2014 at 12:34 PM, Zachary Turner <zturner at google.com> wrote:

> Hmm, that's no good either.  Someone pointed out to me that the reason for
> implementing this as a semaphore probably has to do with the fact that
> std::mutex can be used with a std::condition_variable, and you can't make
> that work with a critical section.  Still, there are better ways to
> implement it, because on Vista+ Windows provides a native condition
> variable object.  Still though, it's unfortunate.
>
> I'm not really sure what the right solution is.  I'll let this thread bake
> for a little while and get some more comments, but I may end up reverting
> this patch if there's no good solution.
>
>
> On Fri, Jun 20, 2014 at 11:39 AM, Vadim Chugunov <vadimcn at gmail.com>
> wrote:
>
>> I suppose there's also an option #3: "declare that LLVM only supports the
>> threads-posix flavor".   As long as configure script gives a clear error
>> message...
>>
>> It should be noted, though, that MinGW's pthreads mutexes are likely to
>> perform worse than LLVM's home-grown ones:
>> http://sourceforge.net/p/mingw-w64/bugs/344/
>>
>>
>>
>> On Fri, Jun 20, 2014 at 11:14 AM, Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> Sorry, I mean only disable this for THREADS-WIN32, not threads-posix.
>>>
>>>
>>> On Fri, Jun 20, 2014 at 11:14 AM, Zachary Turner <zturner at google.com>
>>> wrote:
>>>
>>>> #2 is better if we can detect threads-win32 vs threads-posix on MinGW,
>>>> and only disable this for threads-posix.  We can check for
>>>> _GLIBCXX_HAS_GTHREADS, but that seems somewhat hackish, so I wonder if
>>>> there's a better way.
>>>>
>>>> To handle the switching, I guess we'll have to go back to the original
>>>> option of having llvm::mutex, llvm::recursive_mutex, etc, and then
>>>> conditionally typedefing them.  Kinda sucks, but still better than getting
>>>> rid of it entirely.
>>>>
>>>>
>>>> On Fri, Jun 20, 2014 at 11:09 AM, Reid Kleckner <rnk at google.com> wrote:
>>>>
>>>>> OK, sounds like we're screwed.
>>>>>
>>>>> There's two options:
>>>>> 1. Revert and give up on C++11 threading libraries for now.
>>>>> 2. Do what Eric suggests.  Move all the mutex usage under #ifdef
>>>>> LLVM_ENABLE_THREADS, and disable LLVM_ENABLE_THREADS by default on MinGW.
>>>>>  MinGW plus LLVM_ENABLE_THREADS would become unsupported.
>>>>>
>>>>> Do people have objections to 2?  I don't really like it either.
>>>>>
>>>>>
>>>>> On Fri, Jun 20, 2014 at 10:33 AM, Yaron Keren <yaron.keren at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> The whole "mutex" and "shared_mutex" files are #ifdef
>>>>>> _GLIBCXX_HAS_GTHREADS so if no _GLIBCXX_HAS_GTHREADS there are no mutexes
>>>>>> and no call_once. thread lives in "thread" which is also #ifdef
>>>>>> _GLIBCXX_HAS_GTHREADS.
>>>>>> "condition_variable" and "future" are the same.
>>>>>>
>>>>>> I have tested gcc 4.8.2 predefines and _GLIBCXX_HAS_GTHREADS isn't
>>>>>> there nor it is defined anywhere with the win32 version. I have also
>>>>>> compiled a small test and indeed it failed with
>>>>>>
>>>>>>   a.cpp:4:3: error: 'mutex' is not a member of 'std'.
>>>>>>
>>>>>> Just for fun, I tried to compile it with -D_GLIBCXX_HAS_GTHREADS but
>>>>>> then it failed on bunch of other errors starting with
>>>>>>
>>>>>>   error: '__gthread_time_t' was not declared in this scope
>>>>>>
>>>>>> so gthreads isn't there.
>>>>>>
>>>>>> As to popularity, compare the download graphs for 32 bit:
>>>>>>
>>>>>>
>>>>>> http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/mingw-builds/4.9.0/
>>>>>>
>>>>>> and 64 bit:
>>>>>>
>>>>>>
>>>>>> http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/4.9.0/
>>>>>>
>>>>>> in 32 bit the posix version rules, whereas in 64 bit it is a close
>>>>>> winner. If you go back to 4.8.2 the pattern is similar.
>>>>>>
>>>>>> The win32 version does not support anything thread-related so it's
>>>>>> not C++11 compliant?
>>>>>>
>>>>>> Yaron
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2014-06-20 19:55 GMT+03:00 Reid Kleckner <rnk at google.com>:
>>>>>>
>>>>>>> It sounds like this version of libstdc++ doesn't support
>>>>>>> std::recursive_mutex from C++11.  This is really unfortunate, because we
>>>>>>> were hoping that moving to C++11 would allow us to use standard, portable
>>>>>>> threading primitives.
>>>>>>>
>>>>>>> Does this version of MinGW have any C++11 threading support?  Is it
>>>>>>> just recursive_mutex that is missing, or do we have to avoid std::mutex,
>>>>>>> std::call_once, etc?  lld has been using all of these things for some time
>>>>>>> now, and in theory we have the same baseline toolchain requirements.
>>>>>>>
>>>>>>> If it's just std::recursive_mutex, how long do you think it would
>>>>>>> take to implement that for mingw's libstdc++?
>>>>>>>
>>>>>>> Do you have a sense of which version of mingw is more popular, the
>>>>>>> pthreads variant or the win32 threads variant?  If the overwhelming
>>>>>>> majority use the win32 threads variant, I don't think we can break it.
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Jun 20, 2014 at 9:49 AM, Zachary Turner <zturner at google.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> I kind of feel like we should drop support for this configuration.
>>>>>>>>  Here are the reasons why:
>>>>>>>>
>>>>>>>> 1) clang, lld, and other LLVM-based tools already make use of
>>>>>>>> std::recursive_mutex and std::mutex, so if those types don't exist in this
>>>>>>>> one configuration, we have already (even if inadvertently) made a statement
>>>>>>>> that we don't support that configuration.
>>>>>>>>
>>>>>>>> 2) We chose C++11 as the baseline because all compilers should
>>>>>>>> support it.  This functionality in particular is pretty egregious to not
>>>>>>>> support, considering how simple it is.
>>>>>>>>
>>>>>>>> 3) Not supporting this configuration does not mean we don't support
>>>>>>>> GCC / MinGW, it only means we don't support GCC / MinGW / threads-win32.
>>>>>>>> There is still the threads-posix flavor of this platform which works fine
>>>>>>>> on Windows.
>>>>>>>>
>>>>>>>> #3 is a little unfortunate and backwards, since on Windows we
>>>>>>>> should be encouraging native Windows implementations of things and
>>>>>>>> discouraging posix emulation, but in this case the functionality just isn't
>>>>>>>> implemented.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Fri, Jun 20, 2014 at 9:26 AM, Zachary Turner <zturner at google.com
>>>>>>>> > wrote:
>>>>>>>>
>>>>>>>>> +llvmdev.
>>>>>>>>>
>>>>>>>>> I find this pretty surprising.  Actually, we already use
>>>>>>>>> std::mutex and std::recursive_mutex in clang, lld, and other llvm projects,
>>>>>>>>> it's just a coincidence that it hadn't been introduced into LLVM until my
>>>>>>>>> commits.
>>>>>>>>>
>>>>>>>>> I'm not sure what the right thing to do here is.  If I understand
>>>>>>>>> correctly, it seems like in order to encounter this, a) you must be using
>>>>>>>>> GCC, b) you must be using the MinGW flavor of GCC, and c) you must be using
>>>>>>>>> the threads-win32 flavor of this toolchain.   Only if all 3 of those are
>>>>>>>>> true, then std::mutex and std::recursive_mutex don't exist.
>>>>>>>>>
>>>>>>>>> Anybody else have thoughts on whether this necessitates reverting
>>>>>>>>> the mutex changes, or whether this toolchain configuration should be
>>>>>>>>> supported?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Fri, Jun 20, 2014 at 12:07 AM, Vadim Chugunov <
>>>>>>>>> vadimcn at gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> FYI - this commit broke LLVM build using [[
>>>>>>>>>> http://stackoverflow.com/questions/13212342/whats-the-difference-between-thread-posixs-and-thread-win32-in-gcc-port-of-windo
>>>>>>>>>> | win32 threads ]] flavor of the mingw toolchain.  I am getting [[
>>>>>>>>>> http://stackoverflow.com/questions/14191566/c-mutex-in-namespace-std-does-not-name-a-type
>>>>>>>>>> | error: 'recursive_mutex' in namespace 'std' does not name a type ]].
>>>>>>>>>> Not sure if this would be considered a problem for LLVM...
>>>>>>>>>>
>>>>>>>>>> http://reviews.llvm.org/D4196
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> LLVM Developers mailing list
>>>>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140620/8de22126/attachment.html>

More information about the llvm-dev mailing list