<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;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:"Courier New";}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@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 Renato & Tim,<o:p></o:p></span></p><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>It’s interesting to see there are already different approaches.<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I actually did something similar with your patch #1 (custom lower DIVMOD64) and I couldn’t get it work either. <o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>The problem is legalize occurs earlier.<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>And another tricky thing is the second half of the return value of__aeabi_divmod is needed. <o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>SDIV/UDIV is easier because no matter __adabi_uldivmod or “__udivdi3” is selected, the only difference is the function name. See (LegalizeIntegerTypes.cpp::ExpandIntRes_SDIV(), where a i64 div is converted to function call)<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>I read the other patch (devmod-legal) and seems OK. Why doesn’t it work?<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'> You intercept it in ExpandIntRes_SRM too. <o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>You use getLibcallReturnSize/Value to check if a DivMod is called. <o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>My solution is check if SREMDIV is available, if it is, use it first. This follows the same logic of how SREM_I32 works: if the target provides RTLIB::SDIVREM_XX, then it will be used.<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Although we changed LegalizeIntegerTypes.cpp, the logic actually follows the way SDIV/UDIV works. As long as a target doesn’t fill , RTLIB::SDIVREM_I64 (default is 0 in TargetLoweringBase.cpp), they won’t be affected.<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Thanks,<o:p></o:p></span></pre><pre><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Weiming<o:p></o:p></span></pre><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'>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><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><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"'> Renato Golin [mailto:renato.golin@linaro.org] <br><b>Sent:</b> Monday, November 11, 2013 3:38 PM<br><b>To:</b> Tim Northover<br><b>Cc:</b> Weiming Zhao; llvm-commits<br><b>Subject:</b> Re: [PATCH][ARM]Fix urem/srem of i64 for aeabi target (PR 17881)<o:p></o:p></span></p><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal>Hi Weiming,<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>I worked on this a bit in the past, and have attached both partial solutions to your bug, as well as making my own bug a duplicate of yours, so at least you can understand where we're coming from. Tim helped me review the patches and understand the problem, so both of us will be reviewing this.<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>-renato<o:p></o:p></p></div></div><div><p class=MsoNormal style='margin-bottom:12.0pt'><o:p> </o:p></p><div><p class=MsoNormal>On 11 November 2013 22:03, Tim Northover <<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>> wrote:<o:p></o:p></p><p class=MsoNormal>Hi Weiming,<o:p></o:p></p><div><p class=MsoNormal style='margin-bottom:12.0pt'><br>> Please help to review the patch that addresses i64 mod operation for ARM<br>> aeabi.<o:p></o:p></p></div><p class=MsoNormal>Can this be done in ARM's ISelLowering? The problem is that ExpandRem<br>is *only* the correct implementation for the ARM-specific __aeabi_*<br>functions. The more generic GNU-style calls have a different<br>prototype:<br><br> int64_t __divmoddi4(int64_t l, int64_t r, int64_t *rem);<br><br>As it stands, any other target deciding to Expand divrem is likely to<br>get a nasty shock.<br><br>Cheers.<br><br>Tim.<br>_______________________________________________<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" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p></div></div></body></html>