<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">On Apr 12, 2020, at 2:23 PM, Eli Friedman <<a href="mailto:efriedma@quicinc.com" class="">efriedma@quicinc.com</a>> wrote:<br class=""><div><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">Yes, the llvm::Smart* family of locks still exist.  But very few places are using them outside of MLIR; it’s more common to just use plain std::mutex.<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">That said, I don’t think it’s really a good idea to use them, even if they were fixed to work as designed.  It’s not composable: the boolean “enabled” bit is process-wide, not local to whatever data structure you’re trying to build.  So your single-threaded tool gets some benefit, but the benefit goes away as soon as the process starts using multiple threads, even if there still only one thread using the MLIR context in question.</div></div></div></blockquote><div><br class=""></div><div>Yes, I agree, this is similar to Mehdi’s point.  I think it is clear that “enable” and “disable” multithreaded mode should only be called from applications, not libraries.  Calling one of them from a library breaks composability.</div><br class=""><blockquote type="cite" class=""><div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class="">So probably I’d recommend two things:<o:p class=""></o:p></div><ol start="1" type="1" style="margin-bottom: 0in; margin-top: 0in;" class=""><li class="MsoListParagraph" style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">If locking uncontended locks is showing up on profiles as a performance bottleneck, it’s probably worth looking into ways to reduce that overhead in both single-threaded and multi-threaded contexts. (Reducing the number of locks taken in frequently called code, or using a better lock implementation).<o:p class=""></o:p></li><li class="MsoListParagraph" style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;">If you want some mechanism to disable MLIR locking, it should probably be a boolean attached to the MLIR context in question, not a global variable.<o:p class=""></o:p></li></ol><div style="margin: 0in 0in 0.0001pt; font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></div></div></blockquote><br class=""></div><div>Ok, but let me argue the other way.  We currently have a cmake flag that sets LLVM_ENABLE_THREADS, and that flag enables an across the board speedup.  That cmake flag is the *worst* possible thing for library composability. :-). Are you suggesting that we remove it?</div><div><br class=""></div><div>-Chris</div><br class=""></body></html>