<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Thanks!<div><br></div><div>Committed in r200883.</div><div>PR for the reporting and exhaustive search: <a href="http://llvm.org/bugs/show_bug.cgi?id=18747">http://llvm.org/bugs/show_bug.cgi?id=18747</a></div><div><br></div><div>-Quentin<br><div apple-content-edited="true">
</div>
<br><div><div>On Feb 5, 2014, at 1:20 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">----- Original Message -----<br><blockquote type="cite">From: "Quentin Colombet" <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>><br>To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>Cc: "Commit Messages and Patches for LLVM" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>Sent: Wednesday, February 5, 2014 3:18:15 PM<br>Subject: Re: [PATCH][RegAlloc] Add a last chance recoloring mechanism when everything else failed to find a register<br><br><br><br><br>On Feb 5, 2014, at 1:09 PM, Hal Finkel < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> > wrote:<br><br><br><br>----- Original Message -----<br><br><br>From: "Hal Finkel" < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>To: "Quentin Colombet" < <a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a> ><br>Cc: "Commit Messages and Patches for LLVM" < <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><blockquote type="cite"><br></blockquote>Sent: Wednesday, February 5, 2014 2:14:35 PM<br>Subject: Re: [PATCH][RegAlloc] Add a last chance recoloring mechanism<br>when everything else failed to find a register<br><br>----- Original Message -----<br><br><br>From: "Quentin Colombet" < <a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a> ><br>To: "Hal Finkel" < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>Cc: "Commit Messages and Patches for LLVM"<br>< <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a> >, "Andy Trick" < <a href="mailto:atrick@apple.com">atrick@apple.com</a> >,<br>"Jakob<br>Stoklund Olesen" < <a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a> ><br>Sent: Wednesday, February 5, 2014 1:49:53 PM<br>Subject: Re: [PATCH][RegAlloc] Add a last chance recoloring<br>mechanism when everything else failed to find a register<br><br>Hi Hal,<br><br><br><br>On Feb 5, 2014, at 11:18 AM, Hal Finkel < <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> > wrote:<br><br><br><br>----- Original Message -----<br><br><br>From: "Quentin Colombet" < <a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a> ><br>To: "Jakob Stoklund Olesen" < <a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a> >, "Hal Finkel" <<br><a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> ><br>Cc: "Commit Messages and Patches for LLVM" <<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><br><br>, "Andy Trick" < atrick@apple.com ><br>Sent: Wednesday, February 5, 2014 12:48:55 PM<br>Subject: Re: [PATCH][RegAlloc] Add a last chance recoloring<br>mechanism<br>when everything else failed to find a register<br><br>Hi Hal, Jakob,<br><br>Thanks for the reviews.<br>Attached is the updated patch.<br><br>Hal, just to clarify, this patch does not tackle any spilling<br>problem. This patch “just” introduces a reshuffling (or recoloring)<br>of the registers involved in coloring decisions when we expelled<br>all<br>of the others options (spilling included). This means we may have<br>already done more spilling than necessary, but at least we may be<br>able to allocate the current instance instead of crashing the<br>compiler.<br><br>The two problems are orthogonal. Although I am interested in better<br>spilling heuristics, I do not plan to work on that shortly.<br><br>Okay, well I agree that not crashing is the higher-priority goal ;)<br>-- but in this context, I don't understand what is going on. Unless<br>we have a single instruction that uses and defines more registers<br>than the entire set, how can we exhaust all other options including<br>spilling? It seems like the limiting trivial case is where we spill<br>and reload all relevant registers around every instruction, and<br>that<br>cannot fail.<br><br>Unfortunately, this is not that easy, let me go back to my running<br>example with more details:<br><br>Okay, thanks! I can see, with constraints, how this might happen.<br><br><br><br>Assume the following register class constraints:<br><br>vA can use {R1, R2 }<br><br>vB can use { R2, R3}<br>vC can use {R1 }<br><br><br>Input code:<br>vA = reload<br>vB = reload<br>vC = reload<br>inst vA, vB, vC (three uses, like for a store value, base + offset)<br><br><br>You cannot spill or split vA, vB, and vC, they are as short as<br>possible.<br>A coloring exists for this instance, but the regular heuristic may<br>not find it.<br>For instance, let us say the heuristic assigns the live-ranges<br>top-down in the program order and uses the first available color:<br>vA => R1<br>vB => R2<br>vC => stuck<br>vA and vB were produced by spill, so you cannot do anything here.<br><br><br>If you would have use a graph coloring approach, you would have<br>ended<br>with the same problem. Because of the constrained of the register<br>class, none of the nodes has a degree less than 3, thus you cannot<br>derive a simplification order that is guarantee to complete.<br><br><br>The recoloring approach on the other hand, explore every<br>possibility.<br><br><br>In this particular case, like I said, we end up in that situation<br>because of a lot of factors.<br><br><br><br><br>With regard to the depth cuttoffs, then, if this is a correctness<br>issue, then it seems like instead of skipping/aborting the hard<br>cases, we should queue them to be revisited if no easier solution<br>is<br>found.<br>I prefer not to go in that direction because like I said, this is a<br>backtracking algorithm. Thus if a solution exists we will find it<br>but maybe not in a reasonable amount of time. Thus, we cut the<br>branches that are likely to fail. Also, like Eric mentioned, there<br>may not be a feasible solution with inline asm for instance.<br><br>Okay, I'm not saying that you should not cut the branches on the<br>first pass, but it seems like you should queue them to be visited<br>later. Why do you not want to do this?<br><br>Thinking about it, I suppose the question is what do we want the<br>worst-case behavior to be: crashing, or taking a *long* time (which<br>the user might see as hanging). Perhaps the best solution is<br>something like this:<br><br>- If recoloring fails, we report an error like this (but not crash):<br>Register allocation failed, please retry with<br>-fexhaustive-register-search<br><br><br>- We add support for such an option, and this disables the depth<br>check and other limits<br>I like this alternative.<br>Would it be acceptable that I commit the recoloring approach as it<br>is, file a PR and fix it later?<br></blockquote><br>Yes; sounds good to me.<br><br><blockquote type="cite"><br><br>If we want to explore cheap solutions first this will require a bit<br>of refactoring and I’d like this to land sooner than later if<br>possible.<br><br><br><br><br><br>This will allow the compiler to "work" for all users (even if it<br>takes a long time), but will also allow knowledgeable developers,<br>who will rightfully think this odd, to submit test cases (should<br>this ever happen in practice).<br><br>Alternatively, we could stick the recoloring procedure in a loop with<br>ever-increasing depth bounds.<br><br>What do you think?<br>Nice cross reply :).<br></blockquote><br>:)<br><br>Thanks again,<br>Hal<br><br><blockquote type="cite"><br><br>-Quentin<br><br><br><br><br>-Hal<br><br><br><br><br>-Hal<br><br><br><br><br><br>Cheers,<br>-Quentin<br><br><br><br><br>Thanks again,<br>Hal<br><br><br><br><br>On Feb 5, 2014, at 9:17 AM, Jakob Stoklund Olesen < <a href="mailto:stoklund@2pi.dk">stoklund@2pi.dk</a><br><br><br><br>wrote:<br><br><br><br><br>On Feb 4, 2014, at 3:29 PM, Quentin Colombet < <a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a><br><br><br><br>wrote:<br><br><br><br>Hi Jakob,<br><br>The attached patch introduces a last chance recoloring mechanism<br>when the current allocation scheme fails to assign a register.<br>Thanks for your review.<br><br>** Context **<br><br>In some extreme conditions the current allocation heuristic may<br>fail to find a valid allocation solution whereas one exists.<br>This is demonstrated with the (big) test case that is contained in<br>that patch.<br>Basically, in that test case, the greedy register allocator runs<br>out of registers because of a combination of:<br>- The way the machine scheduler extends some physical register<br>live-ranges, which end up putting a lot of contraints on the<br>available registers.<br>- The relocation model, which consumes one register.<br>- The function attributes, which forces to keep a register for the<br>frame pointer.<br>- The weight of the different variables, which affect the<br>allocation order.<br><br>Hi Quentin,<br><br>The patch looks good to me, but please address Hal’s concerns.<br><br>I can see how this last-chance recoloring can be necessary,<br>particularly when dealing with constrained register classes and<br>inline assembly. However, I am not sure that the machine scheduler<br>is doing the right thing if it is extending physical register live<br>ranges. As I see it, physreg live ranges should always have a COPY<br>in one end, and the copies should be placed to make the live<br>ranges as short as possible.<br>I agree and I was surprised to see such extended live-ranges for<br>physical registers.<br>Like you said and like I showed in my motivating example, the<br>recoloring may still be needed for constrained register classes.<br>Thus, I will pursue with that last chance approach.<br><br>I’ll look into the extended live-ranges problem as some point,<br>because I believe this can improve the quality of the allocation in<br>some cases (here for example :)).<br><br><br><br>Even when the scheduler is tracking register pressure, it is<br>extremely difficult to guarantee that a valid register allocation<br>exists if physical live ranges are extended.<br>Agree.<br><br><br><br>We had this same problem back when the coalescer was extending<br>physreg live ranges, and it was a constant source of obscure “ran<br>out of registers” bugs like your test case.<br><br>Thanks,<br>/jakob<br><br>Thanks again,<br>-Quentin<br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br><br>--<br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory<br><br></blockquote><br>--<span class="Apple-converted-space"> </span><br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory</div></blockquote></div><br></div></body></html>