<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=us-ascii"><meta name=Generator content="Microsoft Word 14 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@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;}
@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.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
.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;}
--></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'>No problem. I will change it.<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'>Another interesting thing is that it uses s16 instead of other callee saved vfp regs. I tried using –O3 and removing “minsize”, still the same. Any ideas?<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><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation</span><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p></o:p></span></p></div><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 #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> Evan Cheng [mailto:evan.cheng@apple.com] <br><b>Sent:</b> Friday, March 14, 2014 12:55 PM<br><b>To:</b> weimingz@codeaurora.org<br><b>Cc:</b> Duncan P. N. Exon Smith; Tim.Northover@arm.com; llvm-commits@cs.uiuc.edu<br><b>Subject:</b> Re: [Patch] [ARM] Fix bug in tryFoldSPUpdateIntoPushPop<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>The name isRegLive() is misleading. Why don't use call it isAnySubRegLive(), which Duncan recommended? Otherwise, the revised patch looks fine to me.<o:p></o:p></p><div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Evan<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>On Mar 14, 2014, at 12:07 PM, Weiming Zhao <<a href="mailto:weimingz@codeaurora.org">weimingz@codeaurora.org</a>> wrote:<o:p></o:p></p></div><p class=MsoNormal><br><br><o:p></o:p></p><div><p class=MsoNormal><span style='font-size:9.0pt'>Hi Duncan,<br><br>Thanks for the comment. You're right. I refactored the code. I upload it to<br><a href="http://llvm-reviews.chandlerc.com/D3085">http://llvm-reviews.chandlerc.com/D3085</a><span class=apple-converted-space> </span>as well.<br><br>Hi Tim,<br><br>Could you help to review the logic?<br><br>Thanks,<br>Weiming<br><br>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by<br>The Linux Foundation<br><br><br>-----Original Message-----<br>From: Duncan P. N. Exon Smith [<a href="mailto:dexonsmith@apple.com">mailto:dexonsmith@apple.com</a>]<span class=apple-converted-space> </span><br>Sent: Thursday, March 13, 2014 10:46 PM<br>To:<span class=apple-converted-space> </span><a href="mailto:weimingz@codeaurora.org">weimingz@codeaurora.org</a><br>Cc:<span class=apple-converted-space> </span><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>Subject: Re: [Patch] [ARM] Fix bug in tryFoldSPUpdateIntoPushPop<br><br><br>On 2014 Mar 13, at 16:14, Weiming Zhao <<a href="mailto:weimingz@codeaurora.org">weimingz@codeaurora.org</a>> wrote:<br><br><br><o:p></o:p></span></p><p class=MsoNormal><span style='font-size:9.0pt'>Hi,<br><br>This patch fixes the issue in<span class=apple-converted-space> </span><br><a href="http://llvm.org/bugs/show_bug.cgi?id=19136">http://llvm.org/bugs/show_bug.cgi?id=19136</a><br>There might be two ways to do it: One is to fix<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:9.0pt'>tryFoldSPUpdateIntoPushPop() to let it check livness of subregs.<br><br><o:p></o:p></span></p><p class=MsoNormal><span style='font-size:9.0pt'>The other ways is to fix MBB->computeRegisterLiveness().<br><br>The patch uses the first way.<br><br>Please help to review.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:9.0pt'><br>diff --git a/lib/Target/ARM/ARMBaseInstrInfo.cpp<br>b/lib/Target/ARM/ARMBaseInstrInfo.cpp<br>index 1aa146d..8a30d3c 100644<br>--- a/lib/Target/ARM/ARMBaseInstrInfo.cpp<br>+++ b/lib/Target/ARM/ARMBaseInstrInfo.cpp<br>@@ -2160,9 +2160,14 @@ bool llvm::tryFoldSPUpdateIntoPushPop(MachineFunction<br>&MF,<br> // registers live within the function we might clobber a return value<br> // register; the other way a register can be live here is if it's<br> // callee-saved.<br>- if (isCalleeSavedRegister(CurReg, CSRegs) ||<br>- MBB->computeRegisterLiveness(TRI, CurReg, MI) !=<br>- MachineBasicBlock::LQR_Dead) {<br>+ bool IsRegLive = false;<br>+ for (MCSubRegIterator Subreg(CurReg, TRI, /* IncludeSelf */true);<br>+ Subreg.isValid() && !IsRegLive; ++Subreg)<br>+ if (MBB->computeRegisterLiveness(TRI, *Subreg, MI) !=<br>+ MachineBasicBlock::LQR_Dead)<br>+ IsRegLive = true;<br>+<br>+ if (isCalleeSavedRegister(CurReg, CSRegs) || IsRegLive) {<br> // VFP pops don't allow holes in the register list, so any skip is<br>fatal<br> // for our transformation. GPR pops do, so we should just keep<br>looking.<br> if (IsVFPPushPop)<br><br>I'm not an expert here, but your change looks unnecessarily bad for compile<br>time. You now call computeRegisterLiveness() even when you don't use the<br>result. You can refactor by moving the calculation into a static<br>function:<br><br> if (isCalleeSavedRegister(CurReg, CSRegs) ||<br> isAnySubRegLive(MBB, CurReg, TRI)) {<br><a.diff>_______________________________________________<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></span></p></div></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></body></html>