Reid Kleckner
2014-Jun-20 16:55 UTC
[LLVMdev] [PATCH] Replace the Execution Engine's mutex with std::recursive_mutex
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 >>> >>> >>> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140620/30d224dc/attachment.html>
Yaron Keren
2014-Jun-20 17:33 UTC
[LLVMdev] [PATCH] Replace the Execution Engine's mutex with std::recursive_mutex
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 > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140620/25c2216b/attachment.html>
Eric Christopher
2014-Jun-20 18:08 UTC
[LLVMdev] [PATCH] Replace the Execution Engine's mutex with std::recursive_mutex
Another thought is that we could introduce a configuration for threading and just conditionalize all of the uses on that and turn it off by default for mingw. If a target doesn’t support threading it doesn’t support it? -eric 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 >
Reid Kleckner
2014-Jun-20 18:09 UTC
[LLVMdev] [PATCH] Replace the Execution Engine's mutex with std::recursive_mutex
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 >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140620/b4c10504/attachment.html>