<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 14 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";
mso-fareast-language:EN-US;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-GB link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Patch committed at r231360.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I also thought about this case and was a bit puzzled to see it did not trigger. I’ll also try it again, just in case I missed something.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Cheers,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Arnaud<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span lang=EN-US style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span lang=EN-US style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> Lang Hames [mailto:lhames@gmail.com] <br><b>Sent:</b> 05 March 2015 05:31<br><b>To:</b> Arnaud De Grandmaison<br><b>Cc:</b> Commit Messages and Patches for LLVM<br><b>Subject:</b> Re: [PBQP] Use a local bit-matrix to speedup searching an edge in the graph<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>Hi Arnaud,<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>This looks great. Please go ahead and commit. :)<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>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.:<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal><span style='font-family:"Courier New"'>A B</span><o:p></o:p></p></div><div><p class=MsoNormal><span style='font-family:"Courier New"'>| <- A (segment 1) joins active set.</span><o:p></o:p></p></div><div><p class=MsoNormal><span style='font-family:"Courier New"'>| | <- B (segment 1) joins active set, interference between A and B detected.</span><o:p></o:p></p></div><div><p class=MsoNormal><span style='font-family:"Courier New"'> <- A and B both leave the active set.</span><o:p></o:p></p></div><div><p class=MsoNormal><span style='font-family:"Courier New"'> | <- B (segment 2) joins the active set.</span><o:p></o:p></p></div><div><p class=MsoNormal><span style='font-family:"Courier New"'>| | <- A (segment 2) joins the active set, interference between A and B (re)detected.</span><o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>It is the second detection that the findEdge call is guarding against.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>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.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>- Lang.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div></div><div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>On Wed, Mar 4, 2015 at 9:05 AM, Arnaud A. de Grandmaison <<a href="mailto:arnaud.degrandmaison@arm.com" target="_blank">arnaud.degrandmaison@arm.com</a>> wrote:<o:p></o:p></p><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Hi Lang,<o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>The attached patch addresses a FIXME in RegAllocPBQP.cpp, in the Interference class when building the graph.<o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>This indeed improves compilation time when building llvm+clang+lldb.<o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>However, I wonder if this check+continue for an already seen edge should exist at all.<o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>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.<o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=FR>Cheers,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=FR>--</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=FR>Arnaud A. de Grandmaison</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span lang=FR> </span><o:p></o:p></p></div></div></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></body></html>