<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:x="urn:schemas-microsoft-com:office:excel" xmlns:p="urn:schemas-microsoft-com:office:powerpoint" xmlns:a="urn:schemas-microsoft-com:office:access" xmlns:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" xmlns:s="uuid:BDC6E3F0-6DA3-11d1-A2A3-00AA00C14882" xmlns:rs="urn:schemas-microsoft-com:rowset" xmlns:z="#RowsetSchema" xmlns:b="urn:schemas-microsoft-com:office:publisher" xmlns:ss="urn:schemas-microsoft-com:office:spreadsheet" xmlns:c="urn:schemas-microsoft-com:office:component:spreadsheet" xmlns:odc="urn:schemas-microsoft-com:office:odc" xmlns:oa="urn:schemas-microsoft-com:office:activation" xmlns:html="http://www.w3.org/TR/REC-html40" xmlns:q="http://schemas.xmlsoap.org/soap/envelope/" xmlns:rtc="http://microsoft.com/officenet/conferencing" xmlns:D="DAV:" xmlns:Repl="http://schemas.microsoft.com/repl/" xmlns:mt="http://schemas.microsoft.com/sharepoint/soap/meetings/" xmlns:x2="http://schemas.microsoft.com/office/excel/2003/xml" xmlns:ppda="http://www.passport.com/NameSpace.xsd" xmlns:ois="http://schemas.microsoft.com/sharepoint/soap/ois/" xmlns:dir="http://schemas.microsoft.com/sharepoint/soap/directory/" xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:dsp="http://schemas.microsoft.com/sharepoint/dsp" xmlns:udc="http://schemas.microsoft.com/data/udc" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:sub="http://schemas.microsoft.com/sharepoint/soap/2002/1/alerts/" xmlns:ec="http://www.w3.org/2001/04/xmlenc#" xmlns:sp="http://schemas.microsoft.com/sharepoint/" xmlns:sps="http://schemas.microsoft.com/sharepoint/soap/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:udcs="http://schemas.microsoft.com/data/udc/soap" xmlns:udcxf="http://schemas.microsoft.com/data/udc/xmlfile" xmlns:udcp2p="http://schemas.microsoft.com/data/udc/parttopart" xmlns:wf="http://schemas.microsoft.com/sharepoint/soap/workflow/" xmlns:dsss="http://schemas.microsoft.com/office/2006/digsig-setup" xmlns:dssi="http://schemas.microsoft.com/office/2006/digsig" xmlns:mdssi="http://schemas.openxmlformats.org/package/2006/digital-signature" xmlns:mver="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns:mrels="http://schemas.openxmlformats.org/package/2006/relationships" xmlns:spwp="http://microsoft.com/sharepoint/webpartpages" xmlns:ex12t="http://schemas.microsoft.com/exchange/services/2006/types" xmlns:ex12m="http://schemas.microsoft.com/exchange/services/2006/messages" xmlns:pptsl="http://schemas.microsoft.com/sharepoint/soap/SlideLibrary/" xmlns:spsl="http://microsoft.com/webservices/SharePointPortalServer/PublishedLinksService" xmlns:Z="urn:schemas-microsoft-com:" xmlns:st="" 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 12 (filtered medium)"><style><!--
/* Font Definitions */
@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:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* 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 bgcolor=white 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'>Attached is the updated iabs.ll test patch for ARM and Thumb to match the new codegen.<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'>Ana.<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'><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, September 30, 2011 9:44 PM<br><b>To:</b> Ana Pazos<br><b>Cc:</b> llvm-commits@cs.uiuc.edu; rajav@codeaurora.org; apazos@codeaurora.org<br><b>Subject:</b> Re: [llvm-commits] LLVM patch to optimize integer ABS idiom for ARM target<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>The patch looks fine to me. But please update the failing tests to match the new codegen. Feel free to get it committed after the update. <o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal style='margin-bottom:12.0pt'>Evan<br><br>On Sep 29, 2011, at 10:01 AM, Ana Pazos <<a href="mailto:apazos@codeaurora.org">apazos@codeaurora.org</a>> wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Hi folks,</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Find attached the updated patch that optimizes integer ABS idiom for the ARM target.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Let me know what you think.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Thank you!</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Ana.</span><o:p></o:p></p><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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> Wednesday, September 28, 2011 5:20 PM<br><b>To:</b> Ana Pazos<br><b>Cc:</b> 'Eli Friedman'; <a href="mailto:rajav@codeaurora.org">rajav@codeaurora.org</a><br><b>Subject:</b> Re: [llvm-commits] LLVM patch to optimize integer ABS idiom for ARM target</span><o:p></o:p></p></div></div><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'>Hi Ana,<o:p></o:p></p><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>The patch looks much cleaner. There are some minor stylistic issues though:<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ BuildMI(*SinkBB, SinkBB->begin(), dl,<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ TII->get(ARM::PHI), ABSDstReg)<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ .addReg(NewRsbDstReg).addMBB(RSBBB)<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ .addReg(NewMovDstReg).addMBB(BB);<o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Indentation.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ if (DisableARMIntABS){<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ return NULL;<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>+ }<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Please make sure there is a space before '{'. Also note in general LLVM code doesn't use { } for single statement if - else clauses.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Please send the patch to llvm-commits for larger audience. Thanks.<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Evan<o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>On Sep 27, 2011, at 6:01 PM, Ana Pazos wrote:<o:p></o:p></p></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><o:p></o:p></p><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Hello Evan,</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I have updated the patch following your comments 1-4 and suggestions 1-2.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I have defined separate ARM ABS and t2ABS nodes and have moved the lowering of these nodes (to movs + rsbmi machine instructions) to pre-RA scheduling time.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The updated patch only modifies ARM target-dependent files.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The "bonuses" features imply changes to LLVM target-independent code which I am not able to address right away.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I have also rerun the llvm/test and llvm/projects/test-suite tests and the results are attached.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Please let me know what you think and if further changes are needed.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Note:</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>- The DAGCombiner lowers SELECT_CC as ASR/ADD/XOR instruction BEFORE each target has a chance to choose its preferred idiom.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The code change in ARMISelDAGToDAG.cpp is to convert ASR/ADD/XOR representing integer ABS into an ARM ABS/t2ABS node.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>This was done to avoid changing LLVM target-independent code.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>- ABS nodes take a source operand and a destination operand (V1 = ABS V0).</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The comparison against 0 and the predicated computation of the absolute value are implicit in the node.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>That is why the ABS nodes are declared as defining CPSR.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Thanks a lot!</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Ana.</span><o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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 #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in;border-width:initial;border-color:initial'><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span class=apple-converted-space><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> </span></span><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>Evan Cheng [mailto:evan.cheng@apple.com]<span class=apple-converted-space> </span><br><b>Sent:</b><span class=apple-converted-space> </span>Sunday, September 18, 2011 11:22 AM<br><b>To:</b><span class=apple-converted-space> </span>Ana Pazos<br><b>Cc:</b><span class=apple-converted-space> </span><a href="mailto:rajav@codeaurora.org">rajav@codeaurora.org</a><br><b>Subject:</b><span class=apple-converted-space> </span>Re: [llvm-commits] LLVM patch to optimize integer ABS idiom for ARM target</span><o:p></o:p></p></div></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>On Sep 16, 2011, at 6:04 PM, Ana Pazos wrote:<o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Hi Evan,</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Thanks for the great feedback.</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I think I can immediately improve the patch by addressing your points 1-4 and suggestions 1-2 and deliver these changes soon.</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The “bonuses” suggestions imply changes to LLVM target-independent code which I might not be able to address right away.</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Would this plan be ok with you?</span><o:p></o:p></p></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Yes. Thanks.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Please see my comments below.</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Talk to you again soon!</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Thanks a lot,</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Ana.</span><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><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 windowtext 3.0pt;padding:3.0pt 0in 0in 0in;border-width:initial;border-color:initial;border-width:initial;border-color:initial;z-index:auto'><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span class=apple-converted-space><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> </span></span><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>Evan Cheng [mailto:evan.cheng@apple.com]<span class=apple-converted-space> </span><br><b>Sent:</b><span class=apple-converted-space> </span>Friday, September 16, 2011 11:32 AM<br><b>To:</b><span class=apple-converted-space> </span>Ana Pazos<br><b>Cc:</b><span class=apple-converted-space> </span>Commit Messages and Patches for LLVM<br><b>Subject:</b><span class=apple-converted-space> </span>Re: [llvm-commits] LLVM patch to optimize integer ABS idiom for ARM target</span><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Hi Ana,<o:p></o:p></p></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Thanks for working on this. It does look like an important peephole optimization. Unfortunately I think this patch has some problems.<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>1. The instruction selection pattern is looking for a very specific case. Does it still work if the source is not a function argument? For example, if it's the return value of a function call, or a result of a computation, I don't think it will work with this patch. <o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] Yes, it still works if the source is not a function argument. Maybe the test case confused you.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The ASR/ADD/XOR pattern that implements integer ABS is searched for in the DAG and can happen in any place in the function code.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Once it is found, it is replaced with the pseudoABS node using as source operand the ADD instruction first source operand (which is the same as SRA instruction first source operand).</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Later when pseudoABS node is lowered, it becomes movs/conditional rsbmi using as source and destination register the pseudoABS instruction destination register.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>So there is nothing in the patch that restricts the operand to be “r0”.</span></i></b><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Ok. I'm just concerned about looking for specific patterns could be fragile which is why I suggest adding a ABS isel opcode.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='color:#1F497D'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>2. MOVrs and RSBccri are not needed. You can expand into MOVr with the optional def set to CPSR and a RSBri with the predicate operand filled in.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] Probably my inexperience with *.td file definitions. I will find out how to use the existing MOVr and RSBri instruction definitions.</span></i></b><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Ok.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>3. We want to avoid pseudo instructions that expands into multiple instructions. As you have noticed, this messes up scheduling. Probably the right solution is to expand the instructions at pre-RA scheduling time.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] I understand why you want to avoid pseudo instructions that expand into multiple instructions. It was my concern too.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I have a version of the patch that implements the optimization at pre-RA scheduling time.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>For that, I had to add a PHI node in the DAG (to be able to add the conditional execution path that computes RSB).</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The problem with this alternative solution is that LLVM decided to spill both input operands to the PHI nodes, and the final code was very inefficient.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I was not using the latest LLVM source code. I will try this solution again with the LLVM svn tip. Will let you know if I see the problem again.</span></i></b><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Ok.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>4. It's a bad idea to have a single instruction that's used for both ARM and Thumb2 mode.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] There is nothing special we need to do handle Thumb2 mode.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I built EEMBC benchmarks in Thumb2 mode and the ABS optimization worked just fine.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Note that in Thumb2 mode conditional execution is implemented with IT instructions.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>There is no need to explicitly write IT instructions though.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The assembler/code emitter inserts it for you where necessary changing the pattern movs rsbmi into movs itmi rsbmi.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>So I am not sure we need t2ABSri type of node.</span></i></b><o:p></o:p></p></div></div></div></div></blockquote><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>You still need pseudo nodes for ARM and Thumb2. We do not allow two modes to share an opcode.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span></i></b><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>My suggestion:<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>1. ISel should match to an instruction (say ABS, t2ABSri) that's marked with usesCustomInserter = 1.<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>2. Add support to lower ABS to MOVr and RSBri with optional def and predicate operands filled in. Please do the same for the Thumb2 variant.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] I think I understood what you meant. Let me give it a try.</span></i></b><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Bonuses:<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>1. Is this sufficient to generate the best code sequence? e.g.<o:p></o:p></p></div></div></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> movs r0, r0<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> rsbmi r0, r0, #0<o:p></o:p></p></div></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>This is the best if r0 is a function input. But how about?<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> add r0, r1<o:p></o:p></p></div></div></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> movs r0, r0<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> rsbmi r0, r0, #0<o:p></o:p></p></div></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Shouldn't we copy propagate the movs?<o:p></o:p></p></div></div></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> adds r0, r1<o:p></o:p></p></div></div></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> rsbmi r0, r0, #0<o:p></o:p></p></div></div></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>One possibility is the scheduler custom expansion code look for the instruction that defines the source and tack the optional def on that instruction. Any other ideas?<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>2. How to make the instruction selection code match more cases? My suggestion is to add a new target independent opcode ABS. For targets where this node is legal, i.e. ARM, dag combine can form this instruction rather than the sra + sub sequence. This way, you can write a simple pattern to match the instruction instead of C++ selection code.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] So instead of ARM/Thumb-specific DAG nodes create a new target independent ABS DAG node…</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>SELECT_CC DAG nodes that represent integer ABS patterns are transformed into target independent ABS DAG nodes.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>And then ARM/Thumb target lowers it as movs/rsbmi and the other targets lower it as sra + add + xor sequence.</span></i></b><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>No, other targets will not generate ABS dag nodes since it will not be legal.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The problem with this solution is that it will imply changes to target-independent LLVM code.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>In my current solution I strived to not change any target-independent LLVM code.</span></i></b><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>I believe this is a less fragile solution.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] Let me give more thought to the 2 bonuses suggestions above.</span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span></i></b><o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><i><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>[apazos] But immediately I can address the other points.</span></i></b><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='color:#1F497D'> </span><o:p></o:p></p></div></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Ok. Thanks.<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Evan<o:p></o:p></p></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><o:p></o:p></p></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>Evan<o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'>On Sep 15, 2011, at 5:32 PM, Ana Pazos wrote:<o:p></o:p></p></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><br><br><br><br><br><o:p></o:p></p></div></div><div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Hello,</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>I worked on an LLVM patch to optimize integer ABS idiom for the ARM target and would like to submit it to your review.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>I experimented with EEMBC benchmarks, in particular MPEG encoding, and noted integer ABS computation happens frequently. Significant speed up was achieved with the optimized idiom for ARM (20% for MPEG encoding).</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Patch details:</span></b><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><b><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span></b><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>LLVM lowers SELECT_CC nodes that represent an integer ABS pattern into ASR/ADD/XOR instructions.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>It is possible to create an optimized machine idiom for integer ABS on ARM formed by MOVs/RSBmi predicated instructions.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>This patch modifies ARM-specific files to implement the above optimized machine idiom for integer ABS.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Generation of the optimized integer ABS idiom is turned on by default. To turn this feature off set -disable-arm-int-abs feature flag.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>abspatch.diff</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Changes to ARM-specific files to implement optimized integer ABS idiom.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>abstestpatch.diff</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>ARM/iabs.ll and Thumb/iabs.ll tests check for the non-optimized integer ABS idiom (ASR/ADD/XOR).</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>When applying abspatch.diff these tests fail. So patched the tests to set -disable-arm-int-abs flag to prevent the compiler from generating optimized integer ABS pattern and allow the non-optimized idiom to be checked.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>iabsopt.ll</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Similar to ARM/iabs.ll and Thumb/iabs.ll tests except that it checks for the optimized integer ABS idiom and checks for all possible test conditions.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>failures.txt</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Failure report and explanation from running llvm/test and projects/test-suite on ARM.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>I noted failures running llvm/test (svn version 139318) and projects/test-suite (svn revision 139319) on ARM. Are these failures expected?</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Thank you,</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'>Ana.</span><o:p></o:p></p></div></div></div><div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><span style='font-size:11.0pt;font-family:"Calibri","sans-serif"'> </span><o:p></o:p></p></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><abspatch.diff><abstestpatch.diff><failures.txt><iabsopt.ll>_______________________________________________<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></div></div></div><div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div></div></div><div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'><abspatch.diff><abstestpatch.diff><failures.txt><iabsopt.ll><o:p></o:p></p></div></div><p class=MsoNormal style='mso-margin-top-alt:auto;mso-margin-bottom-alt:auto'> <o:p></o:p></p></div></div></div></blockquote><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal><abspatch.diff><o:p></o:p></p></div></blockquote><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal><abstestpatch.diff><o:p></o:p></p></div></blockquote><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal><failures.txt><o:p></o:p></p></div></blockquote><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal><iabsopt.ll><o:p></o:p></p></div></blockquote></div></body></html>