<div dir="ltr">Cool test :)<div>It seems to work fine now, I don't see any new failures. IIUC, Kristof is also giving it another run.</div><div><br></div><div>Cheers,</div><div>Diana</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 30 May 2017 at 22:57, Quentin Colombet <span dir="ltr"><<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@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">Hi Diana,<div><br></div><div>I’ve actually gone ahead and pushed the fix as I was able to produce a small reproducer.</div><div><br></div><div>This is <span style="font-family:Menlo;font-size:11px;background-color:rgb(255,255,255)">r304244</span><div><br></div><div>Let me know if you encounter any other problem.</div><div><br></div><div>Cheers,</div><div>-Quentin<br><blockquote type="cite"><div><div class="h5"><div>On May 30, 2017, at 7:42 AM, Quentin Colombet via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="m_1695695160749863078Apple-interchange-newline"></div></div><div><div><div><div class="h5">Thanks Diana.<br><br>That is indeed the assumption in the code and this is obviously wrong.<br><br>Could you try the attached patch?<br><br>(I haven’t even tried to compile it though)<br><br>Cheers,<br>-Quentin<br></div></div><span id="m_1695695160749863078cid:1F6F0F97-86C9-415E-9F99-FC6E97F24E79@apple.com"><localizer_tentative_fix.diff></span><br><blockquote type="cite"><div><div class="h5">On May 30, 2017, at 6:56 AM, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:<br><br>Hi Quentin,<br><br>I've attached a reproducer for the problem.<br><br>I've described what I think the problem is in the file, but the short<br>version is that the localizer shouldn't assume that the iteration<br>order for the uses corresponds to the logical order of instructions in<br>a basic block (we're now localizing before the first use that we find,<br>but that may be later in the basic block, so we'd end up with uses<br>before the def).<br><br>I'm not sure it's possible to test this without running a couple of<br>passes. You might be able to trigger it only with reg bank select +<br>localize, but I haven't tried. Using only the localizer would mean<br>that the iteration order for the uses would be the order in which<br>they're read in, so you wouldn't have this problem.<br><br>Hope that helps,<br>Diana<br><br><br>On 29 May 2017 at 10:06, Diana Picus <<a href="mailto:diana.picus@linaro.org" target="_blank">diana.picus@linaro.org</a>> wrote:<br><blockquote type="cite">Thanks Quentin, it's in progress now, I'll let you know how it goes.<br><br>Cheers,<br>Diana<br><br>On 27 May 2017 at 03:36, Quentin Colombet <<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>> wrote:<br><blockquote type="cite">Hi Kristof,<br><br>I’ve pushed the localizer in r304051 and added it in the AArch64 O0 pipeline<br>in r304052.<br><br>I let Diana investigate the seg fault she was seeing.<br><br>@Diana, let me know if you need help.<br><br>Cheers,<br>-Quentin<br><br>On May 25, 2017, at 1:53 PM, Quentin Colombet via llvm-dev<br><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br><br>Hi Kristof,<br><br>On May 25, 2017, at 2:09 AM, Kristof Beyls <<a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a>> wrote:<br><br><br>On 24 May 2017, at 22:01, Quentin Colombet <<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>> wrote:<br><br>Hi Kristof,<br><br>Thanks for going back so fast!<br><br>On May 24, 2017, at 12:57 PM, Kristof Beyls <<a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a>> wrote:<br><br><br>On 24 May 2017, at 19:31, Quentin Colombet <<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>> wrote:<br><br>Hi Kristof,<br><br>Thanks for the measurements.<br><br>On May 24, 2017, at 6:00 AM, Kristof Beyls <<a href="mailto:kristof.beyls@arm.com" target="_blank">kristof.beyls@arm.com</a>> wrote:<br><br><br>- Comparing against -O0 without globalisel but with the above regalloc<br>options: 5.6% performance drop, 1% code size drop.<br><br>In summary, the measurements indicate some good improvements.<br>I also haven't measure the impact on compile time.<br><br><br>Do you have a mean to make this measurement?<br>Ahmed did a bunch of compile time measurements on our side and I wanted to<br>see if I need to put him on the hook again :).<br><br><br>I did a quick setup with CTMark (part of the test-suite). I ran each of<br>* '-O0 -g',<br>* '-O0 -g -mllvm -global-isel=true -mllvm -global-isel-abort=0', and<br>* '-O0 -g -mllvm -global-isel=true -mllvm -global-isel-abort=0 -mllvm<br>-optimize-regalloc -mllvm -regalloc=greedy'<br>5 times, cross-compiling from X86 to AArch64, and took the median measured<br>compile times.<br>In summary, I see GlobalISel having a compile time that's 3.5% higher than<br>the current -O0 default.<br>With enabling the greedy register allocator, this increases to 28%.<br>28% is probably too high?<br><br><br>I think it is yes.<br>I have attached a quick hack to the greedy allocator to feature a fast mode.<br>Could you give it a try?<br><br>To enable the fast mode, please use (-mllvm) -regalloc-greedy-fast=true<br>(default is false).<br><br><br>I'm afraid it doesn't seem to save much compile time. On geomean, I see<br>about 26% compile time increase against the current -O0 default (compared to<br>28% increase for regalloc greedy without your patch).<br><br><br>Interesting, I guess a lot of time is spent in the coalescer. Could you give<br>a try with -join-liveintervals=false?<br><br><br>With adding -join-liveintervals=false, I see the compile time increase going<br>up to 28% again.<br><br><br>Heh, I am mildly surprised we hand much more live-ranges to the allocator<br>when we do that.<br><br><br><br>Do you know where the time is spent (-time-passes)?<br><br><br>I'm afraid I won't have time to have a closer look in the next couple of<br>days - I don't know where the time is spent at the moment.<br><br><br>Fair enough, will investigate later.<br><br><br><br>Anyhow, fixing all of those, although this is I think the right approach,<br>will take time, so we can go with the localizer.<br><br><br>Right, I don't understand the register allocator well enough to know if that<br>compile time overhead can be fixed, while still getting the necessary<br>codegen benefits the greedy allocator gives.<br>Is there any specific help you're looking for with getting the localizer<br>work well enough for production use?<br><br><br>I’ll clean-up the WIP patch for the localizer, then you guys can fix the bug<br>that you found.<br><br>I’ll do that tomorrow.<br><br>Cheers,<br>-Quentin<br><br><br>Thanks,<br><br>Kristof<br><br><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><br><br></blockquote></blockquote></div></div><localizer-mo-order.mir><br></blockquote><span class=""><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></span></div></div></blockquote></div><br></div></div></blockquote></div><br></div>