<div style="line-height:1.7;color:#000000;font-size:14px;font-family:Arial"><div style="margin: 0;">Hi,</div><div style="margin: 0;"><br></div><div style="margin: 0;">I put my comments inline. thanks.</div><div style="margin: 0;"><br></div><div style="margin: 0;">btw: I found more improve on this patch.</div><div style="margin: 0;"><div style="margin: 0;">+ eor v17.16b, v17.16b, v17.16b</div><div>The clear register operator may replace by MOVI</div></div><p>At 2021-07-03 02:43:07, "Pop, Sebastian" <spop@amazon.com> wrote:</p><blockquote id="isReplyContent" style="PADDING-LEFT: 1ex; MARGIN: 0px 0px 0px 0.8ex; BORDER-LEFT: #ccc 1px solid">
<style><!--
_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;}
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:12.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.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>
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">thanks for your review.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> +#ifdef __MACH__<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> +# define MACH<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> +#else<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> +# define MACH #<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> This is not good idea to bypass .const_data<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">MACH uses ".const_data" directive, which is invalid for ELF.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">For ELF the directive is ".rodata":<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> ELF .section .rodata<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> MACH .const_data<o:p></o:p></span></p><p class="MsoNormal"><span style="font-size:11.0pt"><br></span></p><p class="MsoNormal"><span style="font-size:11.0pt">[MC] I means you may declare MACH_RODATA so similar macro, it is empty on ELF but something on Macho, I guess it better than '#' to bypass unnecessary statement.</span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + ushll v0.8h, v0.8b, #0<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> ...<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + mul v16.8h, v0.8h, v24.8h<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> Why not MULL?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">That would not work for the rest of the computation.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Part of the data in v0 gets used in the next computation,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">and then I would have to split mla into a mull + add.<o:p></o:p></span></p>
<p class="MsoNormal"><br></p><p class="MsoNormal">[MC] This is depends on your algorithm, in your code</p><p class="MsoNormal">below, <span style="font-size: 12pt;">you combin row1 & row2 and multiplier</span></p><p class="MsoNormal"><span style="font-size: 12pt;">coeff[0], however, it also works with 8b x 8b</span></p><p class="MsoNormal"><span style="font-size: 12pt;">with UMULL.</span></p><p class="MsoNormal"><span style="font-size: 12pt;">However, it is a little complex algorithm,</span></p><p class="MsoNormal"><span style="font-size: 12pt;">so we can keep this version and improve in</span></p><p class="MsoNormal"><span style="font-size: 12pt;">future.</span></p><p class="MsoNormal"><span style="font-size: 12pt;">*** Code</span></p><p class="MsoNormal"><span style="font-size: 14.6667px;">> + mul v16.8h, v0.8h, v24.8h</span></p><p class="MsoNormal"><span style="font-size: 14.6667px;">> </span><span style="font-size: 14.6667px;">+ ext v21.16b, v0.16b, v1.16b, #8</span></p><p class="MsoNormal"><span style="font-size: 14.6667px;">> </span><span style="font-size: 14.6667px;">+ mul v17.8h, v21.8h, v24.8h</span></p><p class="MsoNormal"><span style="font-size: 14.6667px;"><o:p></o:p></span></p><p class="MsoNormal"><span style="font-size: 14.6667px;">> </span><span style="font-size: 14.6667px;">+ mov v0.16b, v1.16b</span></p><div>*** End</div><p class="MsoNormal"><span style="font-size:11.0pt"><o:p><br></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + orr v0.16b, v1.16b, v1.16b<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> This is equal to MOV, I guess compiler will replace to right instruction on ARM64<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">I replaced orr with mov instructions.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + // sum row[0-7]<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + dup v18.2d, v16.d[1]<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + dup v19.2d, v17.d[1]<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + add v16.4h, v16.4h, v18.4h<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + add v17.4h, v17.4h, v19.4h<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> + trn1 v16.2d, v16.2d, v17.2d<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">> How about ADDP?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">I replaced the above 5 instructions with the following 3 and the performance improved.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> trn1 v20.2d, v16.2d, v17.2d<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> trn2 v21.2d, v16.2d, v17.2d<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"> add v16.8h, v20.8h, v21.8h<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Please see attached the amended patch.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt">Sebastian<o:p></o:p></span></p>
</div>
</blockquote></div>