<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:Helvetica;
panose-1:2 11 6 4 2 2 2 2 2 4;}
@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;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.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'>Hi Quentin,<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'>Thanks for review. You’re right. It should give up (return false) when condition code update is needed.<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 added two test cases for such scenario.<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'>Thanks,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Weiming<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"'> Quentin Colombet [mailto:qcolombet@apple.com] <br><b>Sent:</b> Thursday, December 05, 2013 3:56 PM<br><b>To:</b> Weiming Zhao<br><b>Cc:</b> llvm-commits<br><b>Subject:</b> Re: [Patch][AArch32] Fix vsel in compareinst optimization<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Hi Weiming,<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The following code does not look correct:<o:p></o:p></p></div><div><p class=MsoNormal>- Sub->getOperand(2).getReg() == SrcReg)<o:p></o:p></p></div><div><p class=MsoNormal>+ Sub->getOperand(2).getReg() == SrcReg && !IsInstrVSel)<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Indeed, if the CMP and SUB have different operands order, the VSEL condition would need to be updated as well if the CMP is removed. Otherwise, we would not select the right operand, would we?<o:p></o:p></p></div><div><p class=MsoNormal>In other words, I think we are missing an update of the opcode of the VSEL in that case, since no condition code is available here.<o:p></o:p></p></div><div><p class=MsoNormal>Am I missing something?<o:p></o:p></p></div><div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>You have a typo in the tests, this happens twice:<o:p></o:p></p></div><div><p class=MsoNormal>+; CHCK-NOT: cmp<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The E is missing.<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><div><div><p class=MsoNormal><span style='font-family:"Helvetica","sans-serif";color:black'>-Quentin<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><div><p class=MsoNormal>On Dec 5, 2013, at 3:19 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><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Hi,<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Attached patch is to fix bug:<span class=apple-converted-space> </span><a href="http://llvm.org/bugs/show_bug.cgi?id=18149"><span style='color:purple'>http://llvm.org/bugs/show_bug.cgi?id=18149</span></a><o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Basically, the cmp optimization assumes a MI that uses CPSR has an immediate MO for ARMCC. But this is not true for VSEL instructions.<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Thanks,<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Weiming<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> <o:p></o:p></span></p></div><p class=MsoNormal><span style='font-size:9.0pt;font-family:"Helvetica","sans-serif"'><0001-Bug-18149-AArch32-VSel-instructions-has-no-ARMCC-fie.patch>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu"><span style='color:purple'>llvm-commits@cs.uiuc.edu</span></a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits"><span style='color:purple'>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</span></a><o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></body></html>