<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 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
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;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle19
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:89937255;
mso-list-type:hybrid;
mso-list-template-ids:-2028696560 80503012 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l0:level1
{mso-level-start-at:0;
mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Calibri",sans-serif;
mso-fareast-font-family:SimSun;
mso-bidi-font-family:"Times New Roman";}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level3
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l0:level4
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level5
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level6
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l0:level7
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l0:level8
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l0:level9
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l1
{mso-list-id:148595551;
mso-list-type:hybrid;
mso-list-template-ids:2121577362 -1085518684 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l1:level1
{mso-level-start-at:0;
mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Calibri",sans-serif;
mso-fareast-font-family:SimSun;
mso-bidi-font-family:"Times New Roman";}
@list l1:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l1:level3
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l1:level4
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l1:level5
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l1:level6
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
@list l1:level7
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Symbol;}
@list l1:level8
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:"Courier New";}
@list l1:level9
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-.25in;
font-family:Wingdings;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></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-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>I agree with Matthias that the Dead flags should be present and correct.<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'>The question is why MachineTraceMetrices is implicitly relying on LiveIntervals.<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'>We can leave the patch as-is for now while we investigate the above question. We can also make AArch64ConditionalCompares requires LI if no one has time/interest in finding the answer.<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'>- Zhaoshi<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><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>From:</span></b><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'> llvm-commits-bounces@cs.uiuc.edu [mailto:llvm-commits-bounces@cs.uiuc.edu] <b>On Behalf Of </b>Matthias Braun<br><b>Sent:</b> Tuesday, June 02, 2015 15:26<br><b>To:</b> Pete Cooper<br><b>Cc:</b> llvm-commits; yi jiang<br><b>Subject:</b> Re: [PATCH] Check Dest Register Liveness using MRI in CondOpt pass<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>On a side note: If a register has no uses then all defs of it must be dead, but you can have multiple definitions and uses and still some of the register definitions can be dead. On the other hand I think the Dead flags on virtual registers are mandatory to be present and correct, this is independent of live intervals being available or not.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>- Matthias<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On Jun 2, 2015, at 1:23 PM, Pete Cooper <<a href="mailto:peter_cooper@apple.com">peter_cooper@apple.com</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>Hi all<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>We’ve discovered that this change (which was meant to be a compile-time improvement only and so NFC) is changing behavior. I’d like to get an idea of how you would all like to proceed.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>So the behavioral difference isn’t in CondOpt, but the next pass called AArch64ConditionalCompares. CondOpt ran first, but AArch64ConditionalCompares was also implicitly using the results of LiveInternvals via MachineTraceMetrics.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>On bzip2 decompress, this patch has resulted in AArch64ConditionalCompares *not* performing a transformation because the metrics suggest its not profitable. This results in performance improving almost 6% on that benchmark.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The specific difference in performance is the branch on line 871 of AArch64ConditionalCompares. Prior to this patch the ResDepth and HeadDepth would be such that its profitable to do the work. After the change, that branch finds its not profitable. Looking in to where we get the data for that branch, MachineTraceMetrics is returning different depths due to the presence of kill/dead flags.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The worrying thing here is that MachineTraceMetrics is implicitly relying on LiveIntervals. It doesn’t need it, but it silently giving different results before/after LI runs isn’t ideal. I don’t know whether to document that, or say its a bug or not.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>I can think of a few solutions, comments welcome on where to go from here.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>1 - Just revert this patch, it wasn’t NFC as it was meant to be.<o:p></o:p></p></div><div><p class=MsoNormal>2 - Make AArch64ConditionalCompares require LiveIntervals. This is an incremental improvement over what we had before as we’ve at least removed one pass being dependent on LI.<o:p></o:p></p></div><div><p class=MsoNormal>3 - Teach MachineTraceMetrics to use MRI as this patch did for CondOpt. This will only work for virtual register dead def checks, so may not be suitable given physical register dead defs and kill flags will still differ depending on LI.<o:p></o:p></p></div><div><p class=MsoNormal>4 - Leave things as they are as we actually have a performance improvement, and file a PR to try work out why AArch64ConditionalCompares can actually slow things down, i.e., reevaluate its heuristics in light of this data.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>BTW, i’ve CCed Gerolf, Yi, and Michael who all did most of the work discovering the change here. They and I are happy to help work out whats going on an how to proceed. We can try provide more data, debug dumps, and IR if needed.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Thanks,<o:p></o:p></p></div><div><p class=MsoNormal>Pete<o:p></o:p></p><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On Apr 22, 2015, at 11:08 AM, Pete Cooper <<a href="mailto:peter_cooper@apple.com">peter_cooper@apple.com</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>Thanks Chad and Zhaoshi.<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>r235532.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Thanks,<o:p></o:p></p></div><div><p class=MsoNormal>Pete<o:p></o:p></p><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On Apr 22, 2015, at 11:02 AM, Chad Rosier <<a href="mailto:mcrosier@codeaurora.org">mcrosier@codeaurora.org</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Zhaoshi gave your patch a LGTM. Go for it. Thanks, Pete.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p></div><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><b><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>From:</span></b><span class=apple-converted-space><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'> </span></span><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Pete Cooper [<a href="mailto:peter_cooper@apple.com">mailto:peter_cooper@apple.com</a>]<span class=apple-converted-space> </span><br><b>Sent:</b><span class=apple-converted-space> </span>Wednesday, April 22, 2015 1:04 PM<br><b>To:</b><span class=apple-converted-space> </span><a href="mailto:mcrosier@codeaurora.org">mcrosier@codeaurora.org</a><br><b>Cc:</b><span class=apple-converted-space> </span><a href="mailto:zhaoshiz@codeaurora.org">zhaoshiz@codeaurora.org</a>; <a href="mailto:apazos@codeaurora.org">apazos@codeaurora.org</a>; Tim Northover; Jiangning Liu; <a href="mailto:sdmitrouk@accesssoftek.com">sdmitrouk@accesssoftek.com</a>; llvm-commits<br><b>Subject:</b><span class=apple-converted-space> </span>Re: [PATCH] Check Dest Register Liveness using MRI in CondOpt pass</span><o:p></o:p></p></div></div></div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>On Apr 22, 2015, at 8:04 AM, Chad Rosier <<a href="mailto:mcrosier@codeaurora.org"><span style='color:purple'>mcrosier@codeaurora.org</span></a>> wrote:<o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Pete,</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>I’ve pinged Zhaoshi and Ana using our internal emails. Hopefully, they can provide some feedback shortly.</span><o:p></o:p></p></div></div></div></blockquote><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>Thanks Chad. Sounds good.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>Pete<br><br><br><o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Chad</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'> </span><o:p></o:p></p></div></div><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><b><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>From:</span></b><span class=apple-converted-space><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'> </span></span><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Pete Cooper [<a href="mailto:peter_cooper@apple.com"><span style='color:purple'>mailto:peter_cooper@apple.com</span></a>]<span class=apple-converted-space> </span><br><b>Sent:</b><span class=apple-converted-space> </span>Tuesday, April 21, 2015 6:51 PM<br><b>To:</b><span class=apple-converted-space> </span><a href="mailto:mcrosier@codeaurora.org"><span style='color:purple'>mcrosier@codeaurora.org</span></a><br><b>Cc:</b><span class=apple-converted-space> </span><a href="mailto:zhaoshiz@codeaurora.org"><span style='color:purple'>zhaoshiz@codeaurora.org</span></a>;<span class=apple-converted-space> </span><a href="mailto:apazos@codeaurora.org"><span style='color:purple'>apazos@codeaurora.org</span></a>; Tim Northover; Jiangning Liu;<span class=apple-converted-space> </span><a href="mailto:sdmitrouk@accesssoftek.com"><span style='color:purple'>sdmitrouk@accesssoftek.com</span></a>; llvm-commits<br><b>Subject:</b><span class=apple-converted-space> </span>[PATCH] Check Dest Register Liveness using MRI in CondOpt pass</span><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>Hi Chad<o:p></o:p></p></div></div><div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'><a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D6048&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=MOz2OsIHlXX17Lrpk-BtwnYKseHBxXVQz_wND6pITU8&s=9B2R2lBWI5AMVkJIuTI3PIwEpEEGpjY-JnEQFmBAk-w&e="><span style='color:purple'>http://reviews.llvm.org/D6048</span></a> (everyone CCed here was CCed on it) added register liveness checking to the CondOpt pass. It does this with<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div></div><blockquote style='margin-left:30.0pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt'><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>if (I->getOperand(0).isDead())<o:p></o:p></p></div></div></div></blockquote><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>However, this requires that some kind of liveness has been run beforehand, so the pass had to require live intervals. Looking at the pass manager dump, we then invalidated live intervals shortly after CondOpt because other passes don’t preserve it.<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>Attached is a patch which removes the dependency on LiveIntervals by checking if the def has any uses. If it has no uses then it was going to be marked dead anyway, so this is equivalent to your original code. I’ve verified that this passes ‘make check’ and specifically stepped through the examples in combine-comparisons-by-cse.ll in lldb to ensure the behavior was the same.<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>When building a bitcode containing all of llc with/without this change, it results in one less run of LiveIntervals per MF and saves 4s out of 80s total compile time.<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>Thanks,<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-line-height-alt:10.5pt'>Pete<o:p></o:p></p></div></div></div></div></div></blockquote></div></div></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></div><p class=MsoNormal>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><o:p></o:p></p></div></blockquote></div><p class=MsoNormal><o:p> </o:p></p></div></body></html>