<div dir="ltr">I'm reviving this thread because I have a new patch ready to go that attempts to push this through again. <div><br></div><div><a href="http://reviews.llvm.org/D4223">http://reviews.llvm.org/D4223</a><br>
</div><div><br></div><div>The last time, there were a couple of major issues that came up during review, outlined in the original post of this thread. All of those have been addressed in already-submitted patches.</div><div>
<br></div><div>One additional concern was expressed offline, which is simply to make sure that this change does not cause a significant performance regression in the single-threaded case. I have run wall-clock comparisons on Windows and Mac OSX of compiling a very large TU that showed no noticeable difference, so I believe that concern has been addressed as well.</div>
<div><br></div><div>Feel free to discuss here or offer comments on the review, but I wanted to make sure the community is aware that I'm attempting to push this through again, in case anyone has concerns that have yet to be addressed.</div>
<div><br></div><div>Zach</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 9, 2014 at 3:58 AM, Mathias Fröhlich <span dir="ltr"><<a href="mailto:Mathias.Froehlich@web.de" target="_blank">Mathias.Froehlich@web.de</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Hi,<br>
<div><div class="h5"><br>
On Saturday, June 07, 2014 10:26:29 Aaron Ballman wrote:<br>
> > 1) Should support multi-threading be a compile-time or runtime parameter in<br>
> > LLVM?<br>
> ><br>
> > Currently it is both. It is compile-time through the use of the define<br>
> > LLVM_ENABLE_THREADS, and it is runtime through the use of functions<br>
> > llvm_start_multithreaded, llvm_is_multithreaded, etc. I and some others<br>
> > feel like runtime support for multi-threading could be removed, and it<br>
> > should be compile-time only. However, I am not aware of all the ways in<br>
> > which this is being used, so this is where I would like some feedback. The<br>
> > issues I have with runtime multithreading support are the following:<br>
> ><br>
> > * It leads to confusing code. At any given point, is multi-threading<br>
> > enabled or disabled? You never know without calling llvm_is_multithreaded,<br>
> > but even calling that is inherently racy, because someone else could disable<br>
> > it after it returns.<br>
> ><br>
> > * It leads to subtle bugs. clang_createIndex, the first time it's called,<br>
> > enables multi-threading. What happens if someone else disables it later?<br>
> > Things like this shouldn't even be possible.<br>
> ><br>
> > * Not all platforms even support threading to begin with. This works now<br>
> > because llvm_start_multithreaded(), if the compile time flag is set to<br>
> > disable threads, simply does nothing. But this decision should be made by<br>
> > someone else. Nobody really checks the return value from<br>
> > llvm_start_multithreaded anyway, so there's already probably bugs where<br>
> > someone tries to start multi-threading, and it fails.<br>
> ><br>
> > * What does it actually mean to turn multi-threading support on and off?<br>
> ><br>
> > Anybody that tries to do this is almost certainly broken due to some edge<br>
> > cases about when it's on and when it's off. So this goes back to the first<br>
> > two points about confusing code and subtle bugs.<br>
><br>
> I agree with your assessment, and my personal feeling is that this<br>
> should be a compile-time switch. Code which attempts to do this at<br>
> runtime often has subtle bugs.<br>
<br>
</div></div>Just to make you aware of a use case scenario that strikes me since some time<br>
together with the open source OpenGL driver stack that makes use of llvm.<br>
<br>
There you see driver modules which potentially get loaded anywhere at<br>
application run time. Potentially this can happen from any thread (even if in<br>
reality probably serialized by the dynamic loader) and thus can also race against<br>
the main application itself calling llvm_start_multithreaded in an other thread.<br>
Or if I remember correctly as documented, llvm_start_multithreaded must not be<br>
called concurrently to any other llvm api calls possibly accessing any of<br>
the llvm singeltons. Which in turn is hard to guarantee if you get dynamically<br>
loaded at run time and you have to expect the main application to use llvm already.<br>
<br>
Given that I would wish that you can just make use of llvm concurrently and having<br>
that no option at all, but I can easily anticipate the usual resistance against that.<br>
<br>
Nevertheless if multithreading is a compile option, it would be great if a user<br>
of llvm can see if the llvm libraries have been compiled with multithreading<br>
enabled or not. Probably best queriable at runtime so that a potential<br>
user can bail out in a nice way instead of just crashing later ...<br>
<br>
So it would be good if you keep this use case in mind when doing changes in this corner!<br>
<br>
Thanks<br>
<span class="HOEnZb"><font color="#888888"><br>
Mathias<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
</div></div></blockquote></div><br></div>