<div dir="ltr">Hi Arnaud,<div><br></div><div>This looks great. Please go ahead and commit. :)</div><div><br></div><div>I'm very surprised to hear that the llvm_unreachable never triggered. I will have to have a closer look at that. The Interference::apply algorithm is derived from Linear Scan. It walks the segments of the live-intervals of the function according to their starting points and maintains a set (the active set) of the live segments at the current point. The list of segments whose start point is further down in the function is called the inactive set. A duplicate interference detection should occur any time two intervals have separate overlapping segments. E.g.:</div><div><br></div><div><font face="monospace, monospace">A B</font></div><div><font face="monospace, monospace">|   <- A (segment 1) joins active set.</font></div><div><font face="monospace, monospace">| | <- B (segment 1) joins active set, interference between A and B detected.</font></div><div><font face="monospace, monospace">    <- A and B both leave the active set.</font></div><div><font face="monospace, monospace">  | <- B (segment 2) joins the active set.</font></div><div><font face="monospace, monospace">| | <- A (segment 2) joins the active set, interference between A and B (re)detected.</font></div><div><br></div><div>It is the second detection that the findEdge call is guarding against.</div><div><br></div><div>I'll see if I can replicate your results in the next few days. I want to revisit this code anyway: I have a suspicion that there are further performance improvements to be had, though I don't know if they'll be significant.</div><div><br></div><div>- Lang.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 4, 2015 at 9:05 AM, Arnaud A. de Grandmaison <span dir="ltr"><<a href="mailto:arnaud.degrandmaison@arm.com" target="_blank">arnaud.degrandmaison@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-GB" link="blue" vlink="purple"><div><p class="MsoNormal">Hi Lang,<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">The attached patch addresses a FIXME in RegAllocPBQP.cpp, in the Interference class when building the graph.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">This indeed improves compilation time when building llvm+clang+lldb.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">However, I wonder if this check+continue for an already seen edge should exist at all.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal">I may have missed something in my reading / understanding of Interference::apply(), but I do not think the algorithm can exhibit such a case --- or it would be an error, which we should trap with an assert, either in Interference::apply, or even better in Graph::addEdge. I replaced the continue statement with an llvm_unreachable, and it never triggered while building llvm+clang+lldb (granted, this is not a proof, just an hint). Removing this check+continue brings yet another nice speedup, making pbqp build llvm+clang+lldb slightly faster than the default allocator in release mode.<u></u><u></u></p><p class="MsoNormal"><u></u> <u></u></p><p class="MsoNormal"><span lang="FR">Cheers,<u></u><u></u></span></p><p class="MsoNormal"><span lang="FR">--<u></u><u></u></span></p><p class="MsoNormal"><span lang="FR">Arnaud A. de Grandmaison<u></u><u></u></span></p><p class="MsoNormal"><span lang="FR"><u></u> <u></u></span></p></div></div></blockquote></div><br></div>