<div dir="ltr">Is <a href="https://reviews.llvm.org/differential/diff/100829/" target="_blank" class="gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed cremed" style="font-size:12.8px">https://reviews.llvm.org/<wbr>differential/diff/100829/</a><span style="font-size:12.8px"> replacement for </span><span style="font-size:12.8px">r303341?</span><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">If so LGTM.</span><br><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">r303542 </span>msan AArch64InstructionSelector.cpp: <span style="font-size:12.8px">1m17.209s</span><span style="font-size:12.8px"><br></span></div><div><div><span style="font-size:12.8px">r303542+</span><a href="https://reviews.llvm.org/differential/diff/100829/" target="_blank" class="gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed gmail-cremed cremed" style="font-size:12.8px">diff/100829/</a> msan AArch64InstructionSelector.cpp: 1m24.724s<br></div><div><br></div></div><div><span style="font-size:12.8px"><br></span></div><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 31, 2017 at 6:13 AM, Daniel Sanders <span dir="ltr"><<a href="mailto:daniel_l_sanders@apple.com" target="_blank">daniel_l_sanders@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Great! I expect I'll be able to cut it down further once I start fusing these smaller state-machines together. Before that, I'll re-order the patches that went into that diff so that I don't have to re-commit the regression before fixing it.<div><div class="h5"><div><div><div><br><div><blockquote type="cite"><div>On 31 May 2017, at 13:48, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:</div><br class="m_-5817535166028216979Apple-interchange-newline"><div><div dir="ltr"><div>Hi,</div><div><br></div>This runs in:<div><div>real 13m6.296s</div><div>user 42m45.191s</div><div>sys 1m2.030s</div></div><div><br></div><div>(on top of a fully built r303542). It should be fine for the ARM bots.</div><div><br></div><div>However, you need to 'return std::move(M)' at line 1884.</div><div><br></div><div>@Vitaly, is it ok for your bots as well?</div><div><br></div><div>Cheers,</div><div>Diana</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 31 May 2017 at 10:21, Daniel Sanders <span dir="ltr"><<a href="mailto:daniel_l_sanders@apple.com" target="_blank">daniel_l_sanders@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Hi Diana and Vitaly,<div><br></div><div>Could you give <a href="https://reviews.llvm.org/differential/diff/100829/" target="_blank">https://reviews.llvm.org/<wbr>differential/diff/100829/</a> a try? When measuring the compile of AArch64InstructionSelector.cpp<wbr>.o with asan enabled and running under instruments's Allocation profiler, my machine reports that the cumulative memory allocations is down to ~3.5GB (was ~10GB), the number of allocations down to ~4 million (was ~23 million), and the compile time down to ~15s (was ~60s).</div><div><br></div><div>The patch is based on r303542 and the main change is that most of the generated C++ has been replaced with a state-machine based implementation. It's not fully converted to a state-machine yet since it generates lots of smaller machines (one matcher and one emitter per rule) instead of a single machine but it's hopefully sufficient to unblock my patch series.</div><div><div class="m_-5817535166028216979h5"><div><br></div><div><div><blockquote type="cite"><div>On 26 May 2017, at 09:10, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:</div><br class="m_-5817535166028216979m_-1218326532135719949Apple-interchange-newline"><div><div>Ok, that sounds reasonable. I'm happy to test more patches for you<br>when they're ready.<br><br>On 25 May 2017 at 17:39, Daniel Sanders <<a href="mailto:daniel_l_sanders@apple.com" target="_blank">daniel_l_sanders@apple.com</a>> wrote:<br><blockquote type="cite">Thanks for trying that patch. I agree that 34 mins still isn't good enough but we're heading in the right direction.<br><br>Changing the partitioning predicate to the instruction opcode rather than the number of operands in the top-level instruction will hopefully cut it down further. I also have a patch that shaves a small amount off of the compile-time by replacing the various LLT::scalar()/LLT::vector() calls with references to LLT objects that were created in advance. I tried something similar with the getRegBankForRegClass() but I haven't written that as a patch yet since that one requires some refactors to get access to a mapping that RegisterBankEmitter.cpp knows. In my experiment I edited this information into AArchGenGlobalISel.inc by hand.<br><br>I think the real solution is to convert the generated C++ to the state-machine that we intended to end up with. I don't think we'll be able to put it off much longer given that we're hitting compile-time problems when we can only import 25% of the rules. That said, I have a couple more nearly-finished patches I'd like to get in before we introduce the state machine. Hopefully, the above tricks will be enough to save me a re-write.<br><br><blockquote type="cite">On 25 May 2017, at 16:11, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:<br><br>Hi Daniel,<br><br>I built r303542, then applied your patch and built again and it still takes<br>real 34m30.279s<br>user 84m36.553s<br>sys 0m58.372s<br><br>This is better than the 50m I saw before, but I think we should try to<br>make it a bit faster. Do you have any other ideas to make it work?<br><br>Thanks,<br>Diana<br><br><br>On 22 May 2017 at 11:22, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:<br><blockquote type="cite">Hi Daniel,<br><br>I did your experiment on a TK1 machine (same as the bots) and for r303258 I get:<br>real 18m28.882s<br>user 35m37.091s<br>sys 0m44.726s<br><br>and for r303259:<br>real 50m52.048s<br>user 88m25.473s<br>sys 0m46.548s<br><br>If I can help investigate, please let me know, otherwise we can just<br>try your fixes and see how they affect compilation time.<br><br>Thanks,<br>Diana<br><br>On 22 May 2017 at 10:49, Daniel Sanders <<a href="mailto:daniel_l_sanders@apple.com" target="_blank">daniel_l_sanders@apple.com</a>> wrote:<br><blockquote type="cite">r303341 is the re-commit of the r303259 which tripled the number of rules<br>that can be imported into GlobalISel from SelectionDAG. A compile time<br>regression is to be expected but when I looked into it I found it was ~25s<br>on my machine for the whole incremental build rather than the ~12mins you<br>are seeing. I'll take another look.<br><br>I'm aware of a couple easy improvements we could make to the way the<br>importer works. I was leaving them until we change it over to a state<br>machine but the most obvious is to group rules by their top-level gMIR<br>instruction. This would reduce the cost of the std::sort that handles the<br>rule priorities in generating the source file and will also make it simpler<br>for the compiler to compile it.<br><br><br>On 21 May 2017, at 11:16, Vitaly Buka <<a href="mailto:vitalybuka@google.com" target="_blank">vitalybuka@google.com</a>> wrote:<br><br>It must be r303341, I commented on corresponding llvm-commits thread.<br><br>On Fri, May 19, 2017 at 7:34 AM, Diana Picus via llvm-dev<br><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br><blockquote type="cite"><br>Ok, thanks. I'll try to do a bisect next week to see if I can find it.<br><br>Cheers,<br>Diana<br><br>On 19 May 2017 at 16:29, Daniel Sanders <<a href="mailto:daniel_l_sanders@apple.com" target="_blank">daniel_l_sanders@apple.com</a>><br>wrote:<br><blockquote type="cite"><br><blockquote type="cite">On 19 May 2017, at 14:54, Daniel Sanders via llvm-dev<br><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br><br>r303259 will have increased compile-time since it tripled the number of<br>importable<br>SelectionDAG rules but a quick measurement building the affected file:<br> ninja<br>lib/Target/<Target>/CMakeFiles<wbr>/LLVM<Target>CodeGen.dir/<<wbr>Target>InstructionSelector.<wbr>cpp.o<br>for both ARM and AArch64 didn't show a significant increase. I'll check<br>whether<br>it made a different to linking.<br></blockquote><br>I don't think it's r303259. Starting with a fully built r303259, then<br>updating to r303258 and running 'ninja' gives me:<br> real 2m28.273s<br> user 13m23.171s<br> sys 0m47.725s<br>then updating to r303259 and running 'ninja' again gives me:<br> real 2m19.052s<br> user 13m38.802s<br> sys 0m44.551s<br><br><blockquote type="cite">sanitizer-x86_64-linux-fast also timed out after one of my commits this<br>morning.<br><br><blockquote type="cite">On 19 May 2017, at 14:14, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:<br><br>Hi,<br><br>We've noticed that recently some of our bots (mostly<br>clang-cmake-armv7-a15 and clang-cmake-thumbv7-a15) started timing out<br>whenever someone commits a change to TableGen:<br><br>r303418:<br><a href="http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/7268" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-cmake-armv7-a15/<wbr>builds/7268</a><br>r303346:<br><a href="http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/7242" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-cmake-armv7-a15/<wbr>builds/7242</a><br>r303341:<br><a href="http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/7239" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-cmake-armv7-a15/<wbr>builds/7239</a><br>r303259:<br><a href="http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/7198" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-cmake-armv7-a15/<wbr>builds/7198</a><br><br>TableGen changes before that (I checked about 3-4 of them) don't have<br>this problem:<br>r303253:<br><a href="http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/7197" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/clang-cmake-armv7-a15/<wbr>builds/7197</a><br><br>That one in particular actually finishes the whole build in 635s,<br>which is only a bit over 50% of the timeout limit (1200s). So, between<br>r303253 and now, something happened that made full builds<br>significantly slower. Does anyone have any idea what that might have<br>been? Also, has anyone noticed this on other bots?<br><br>Thanks,<br>Diana<br></blockquote><br>______________________________<wbr>_________________<br>LLVM Developers mailing list<br><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br></blockquote><br></blockquote>______________________________<wbr>_________________<br>LLVM Developers mailing list<br><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br></blockquote><br><br><br></blockquote></blockquote></blockquote><br></blockquote></div></div></blockquote></div><br></div></div></div></div></blockquote></div><br></div>
</div></blockquote></div><br></div></div></div></div></div></div></blockquote></div><br></div>