<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 5, 2015 at 6:15 AM, Peter Collingbourne <span dir="ltr"><<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">LTOModule owns a Module under the covers, </blockquote><div><br></div><div>Does it need to/does it (need to) use it for LLD's uses? Perhaps we could expose that as an actual stateful object rather than global mutable/shared state?<br></div><div><br></div><div>Ah, I see - it gets the "global" LLVMContext - it doesn't even matter about the Module that LTOModule might have.<br><br>Rui: Does this code need to create the Modules (that it's creating in parallel) all in the same LLVMContext? I imagine it probably does, in which case I guess there's currently nothing to do but to lock/only build one Module at a time. Probably.<br><br>- David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">so any client that creates multiple<br>
LTOModules in the same LLVMContext will be affected.<br>
<br>
Maybe it's time for something like an LLVMContext-wide mutex, but that would<br>
present its own design issues (e.g. we would need to define exactly where<br>
guards should go) so I'd prefer to let clients worry about this until we<br>
have a good sense of the right design.<br>
<br>
Peter<br>
<div class="HOEnZb"><div class="h5"><br>
On Sat, Sep 19, 2015 at 06:22:47PM -0700, Rui Ueyama via llvm-commits wrote:<br>
> I have no idea. CC'ing Peter since he may know about that.<br>
><br>
> On Sat, Sep 19, 2015 at 5:31 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> ><br>
> ><br>
> > On Sat, Sep 19, 2015 at 4:14 PM, Rui Ueyama via llvm-commits <<br>
> > <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br>
> ><br>
> >> Author: ruiu<br>
> >> Date: Sat Sep 19 18:14:51 2015<br>
> >> New Revision: 248102<br>
> >><br>
> >> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=248102&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=248102&view=rev</a><br>
> >> Log:<br>
> >> COFF: Fix thread-safety bug.<br>
> >><br>
> >> LTOModule doesn't seem to be thread-safe, so guard that with mutex.<br>
> >><br>
> ><br>
> > Yeah, that seems unfortunate.<br>
> ><br>
> > Any idea where the thread unsafety comes from & whether we can/should fix<br>
> > it down the stack rather than in lld?<br>
> ><br>
> ><br>
> >><br>
> >> Modified:<br>
> >> lld/trunk/COFF/InputFiles.cpp<br>
> >> lld/trunk/COFF/InputFiles.h<br>
> >><br>
> >> Modified: lld/trunk/COFF/InputFiles.cpp<br>
> >> URL:<br>
> >> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=248102&r1=248101&r2=248102&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.cpp?rev=248102&r1=248101&r2=248102&view=diff</a><br>
> >><br>
> >> ==============================================================================<br>
> >> --- lld/trunk/COFF/InputFiles.cpp (original)<br>
> >> +++ lld/trunk/COFF/InputFiles.cpp Sat Sep 19 18:14:51 2015<br>
> >> @@ -310,6 +310,9 @@ void ImportFile::parse() {<br>
> >> }<br>
> >><br>
> >> void BitcodeFile::parse() {<br>
> >> + // Usually parse() is thread-safe, but bitcode file is an exception.<br>
> >> + std::lock_guard<std::mutex> Lock(Mu);<br>
> >> +<br>
> >> std::string Err;<br>
> >> M.reset(LTOModule::createFromBuffer(MB.getBufferStart(),<br>
> >> MB.getBufferSize(),<br>
> >> @@ -356,5 +359,7 @@ MachineTypes BitcodeFile::getMachineType<br>
> >> }<br>
> >> }<br>
> >><br>
> >> +std::mutex BitcodeFile::Mu;<br>
> >> +<br>
> >> } // namespace coff<br>
> >> } // namespace lld<br>
> >><br>
> >> Modified: lld/trunk/COFF/InputFiles.h<br>
> >> URL:<br>
> >> <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=248102&r1=248101&r2=248102&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/InputFiles.h?rev=248102&r1=248101&r2=248102&view=diff</a><br>
> >><br>
> >> ==============================================================================<br>
> >> --- lld/trunk/COFF/InputFiles.h (original)<br>
> >> +++ lld/trunk/COFF/InputFiles.h Sat Sep 19 18:14:51 2015<br>
> >> @@ -17,6 +17,7 @@<br>
> >> #include "llvm/Object/COFF.h"<br>
> >> #include "llvm/Support/StringSaver.h"<br>
> >> #include <memory><br>
> >> +#include <mutex><br>
> >> #include <set><br>
> >> #include <vector><br>
> >><br>
> >> @@ -213,6 +214,7 @@ private:<br>
> >> std::vector<SymbolBody *> SymbolBodies;<br>
> >> llvm::BumpPtrAllocator Alloc;<br>
> >> std::unique_ptr<LTOModule> M;<br>
> >> + static std::mutex Mu;<br>
> >> };<br>
> >><br>
> >> } // namespace coff<br>
> >><br>
> >><br>
> >> _______________________________________________<br>
> >> llvm-commits mailing list<br>
> >> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> >> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
> >><br>
> ><br>
> ><br>
<br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Peter<br>
</font></span></blockquote></div><br></div></div>