<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Apr 12, 2020 at 1:17 PM Chris Lattner <<a href="mailto:clattner@nondot.org">clattner@nondot.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div><br><blockquote type="cite"><div>On Apr 12, 2020, at 11:27 AM, Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:</div><br><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hey Chris,<div><br></div><div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Apr 11, 2020 at 5:15 PM Chris Lattner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Hi all,<div><br></div><div>I was looking at the profile for a tool I’m working on, and noticed that it is spending 10% of its time doing locking related stuff.  The structure of the tool is that it reading in a ton of stuff (e.g. one moderate example I’m working with is 40M of input) into MLIR, then uses its multithreaded pass manager to do transformations.</div><div><br></div><div>As it happens, the structure of this is that the parsing pass is single threaded, because it is parsing through a linear file (the parser is simple and fast, so this is bound by IR construction).  This means that none of the locking during IR construction is useful.</div></div></blockquote><div><br></div><div>I'm curious which are the places that show up on the profile? Do you have a few stacktraces to share?</div></div></div></div></div></div></div></div></blockquote><div><br></div><div>In my case, it is all MLIR attribute/type uniquification stuff which is guarded by a RWMutex.</div><br><blockquote type="cite"><div><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div>Historically, LLVM had a design where you could dynamically enable and disable multithreading support in a tool, which would be perfect for this use case, but it got removed by <a href="https://github.com/llvm/llvm-project/commit/9c9710eaf4c1f01b8b518bdba89aba059ab14175#diff-bb772a7e6d4ebf2b46c6d42c95f40ddf" target="_blank">this patch</a>: (xref <a href="https://reviews.llvm.org/D4216" target="_blank">https://reviews.llvm.org/D4216</a>).  The rationale in the patch doesn’t make sense to me - this mode had nothing to do with the old LLVM global lock, this had to do with whether llvm::llvm_is_multithreaded() returned true or false … which all the locking stuff is guarded on.</div></div></blockquote><div><br></div><div>It seems that at the time the assumption was that this flag was there to alleviate the cost of the global lock only and removing the lock removed the motivation for the feature? Looks like you proved this wrong :)</div><div><br></div><div>+Zach, David, and Reid to make sure they don't miss this.<br></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Yeah, it was about not paying the cost for synchronization when it wasn’t worthwhile.</div><br><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div>Would it make sense to re-enable this, or am I missing something?</div></div></blockquote><div><br></div><div>Finding a way to re-enable it seems interesting. I wonder how much it'll interact with the places inside the compiler that are threaded now, maybe it isn't much more than tracking and auditing the uses of LLVM_ENABLE_THREADS (like lib/Support/ThreadPool.cpp for example). Have you already looked into it?</div></div></div></div></div></div></div></blockquote><br></div><div>It is super-easy to reenable, because the entire codebase is still calling llvm::llvm_is_multithreaded().  We just need to add the global back, along with the methods to set and clear the global, and change llvm::llvm_is_multithreaded() to something like:</div><div><br></div><div><div style="background-color:rgb(255,255,255);line-height:18px;white-space:pre-wrap"><div><span style="color:rgb(0,0,255)">bool</span> llvm::llvm_is_multithreaded() {</div><div><span style="color:rgb(0,0,255)">#if LLVM_ENABLE_THREADS </span>!=<span style="color:rgb(0,0,255)"> </span><span style="color:rgb(9,134,88)">0</span></div><div>  <span style="color:rgb(0,0,255)">return</span> <font color="#0000ff"><span>someGlobal</span></font>;</div><div><span style="color:rgb(0,0,255)">#else</span></div><div>  <span style="color:rgb(0,0,255)">return</span> <span style="color:rgb(0,0,255)">false</span>;</div><div><span style="color:rgb(0,0,255)">#endif</span></div><div>}</div><br></div></div></div></blockquote><div><br></div><div>This is the part I am not sure about: the ThreadPool I mentioned above for example is not checking llvm_is_multithreaded() I believe, and I doubt that the client of the ThreadPool are. So you can have multiple things in flight in the ThreadPool that relies on llvm::Mutex and similar things to operate properly. </div><div>Seems like we could end-up in a situation where llvm_is_multithreaded() is returning false, effectively disabling all the mutex and other safety, while some code would use the ThreadPool.</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div></div></div></div></div>