<font face="arial" size="2"><p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;">Cool, looking into that as an option, sounds ideal. Thanks for the steer. :)</p>
<p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"> </p>
<p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;">I'll get back to you guys with results and a patch (probably via the maintainer, Dylan).</p>
<p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"> </p>
<p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;">Carl</p>
<p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"> </p>
<p style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;">-----Original Message-----<br />From: "Eli Friedman" <efriedma@quicinc.com><br />Sent: Tuesday, February 19, 2019 3:44pm<br />To: "carl-llvm-dev@petosoft.com" <carl-llvm-dev@petosoft.com>, "LLVM Development List" <llvm-dev@lists.llvm.org><br />Subject: RE: [llvm-dev] AVR is little endian, but requires function arguments to be in a "big endian" order, might need an additional data layout variable unless someone can suggest a better fix?<br /><br /></p>
<style><!--
#SafeStyles1550657742 @font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
#SafeStyles1550657742 @font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
#SafeStyles1550657742 p.MsoNormal, #SafeStyles1550657742 li.MsoNormal, #SafeStyles1550657742 div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
#SafeStyles1550657742 a:link, #SafeStyles1550657742 span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
#SafeStyles1550657742 a:visited, #SafeStyles1550657742 span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
#SafeStyles1550657742 p.msonormal0, #SafeStyles1550657742 li.msonormal0, #SafeStyles1550657742 div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
#SafeStyles1550657742 span.EmailStyle20
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
#SafeStyles1550657742 .MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
#SafeStyles1550657742 @page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
#SafeStyles1550657742 div.WordSection1
{page:WordSection1;}
--></style>
<div id="SafeStyles1550657742">
<div class="WordSection1">
<p class="MsoNormal" style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;">Adding a method to TargetLowering makes sense, sure. (I don’t think we need to modify the IR because the difference doesn’t affect IR optimizations.)</p>
<p class="MsoNormal" style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"> </p>
<p class="MsoNormal" style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;">-Eli</p>
<p class="MsoNormal" style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"> </p>
<div style="border: none; border-left: solid blue 1.5pt; padding: 0in 0in 0in 4.0pt;">
<div>
<div style="border: none; border-top: solid #E1E1E1 1.0pt; padding: 3.0pt 0in 0in 0in;">
<p class="MsoNormal" style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"><strong>From:</strong> llvm-dev <llvm-dev-bounces@lists.llvm.org> <strong>On Behalf Of </strong>carl-llvm-dev@petosoft.com via llvm-dev<br /><strong>Sent:</strong> Tuesday, February 19, 2019 12:38 PM<br /><strong>To:</strong> LLVM Developers Mailing List <llvm-dev@lists.llvm.org><br /><strong>Subject:</strong> [EXT] [llvm-dev] AVR is little endian, but requires function arguments to be in a "big endian" order, might need an additional data layout variable unless someone can suggest a better fix?</p>
</div>
</div>
<p class="MsoNormal" style="margin:0;padding:0;font-family: arial; font-size: 10pt; word-wrap: break-word;"> </p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">I think this is broken in at least one place when legalising the DAG.</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">This llvm ir:<br /> %3 = call { i16, i1 } @llvm.umul.with.overflow.i16(i16 %2, i16 11)</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">Fails to lower correctly on AVR but the problem is, unfortunately, not just coming from the AVR Target code and I am not sure it can be cleanly fixed just there. (But I would be very happy to be proved wrong as I'm very new to this.)</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;"><br /> The above code is taken by the legalizer and turned into a call to __mulsi3, but turns the 16 bit parameters into 32 bit parameters by adding 16 bits of 0 to the top 16 bits of each.</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">Unfortunately, to do this, it changes the code to be a call to __mulsi3 with four 16 bit parameters instead of two 32 bit parameters. It orders these parameters based on the endianness of the platform, read from the data layout. Unfortunately that doesn't match the AVR ABI, which expects 32 bit parameters to be passed in four consecutive registers, yet generally takes parameters in a sort of "big endian" order.</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">Details can be found on my bug report to the AVR team: <a href="https://github.com/avr-rust/rust/issues/129">https://github.com/avr-rust/rust/issues/129</a></span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">In order to get around this, the only way to deal with this I can think of is to add an extra field to the data layout for ABIEndianness or something like that, because otherwise the information that these were 32 bit parameters, not two 16 bit parameters has been lost after legalization.</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;"><br /> Any help much appreciated!</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;"> </span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">Thanks,</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">Carl</span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;"> </span></p>
<p style="margin:0;padding:0;margin: 0in; margin-bottom: .0001pt; overflow-wrap: break-word;"><span style="font-size: 10.0pt; font-family: 'Arial',sans-serif;">p.s. I came up with a sample/demo PR for the sort of thing I'm talking about. Definitely not a finished product, just for discussion: <a href="https://github.com/avr-rust/llvm/pull/8">https://github.com/avr-rust/llvm/pull/8</a>.</span></p>
</div>
</div>
</div></font>