<div dir="ltr">Hi Tim,<br><br>I changed my patch by:<div><br>*Change custom promotion to pattern match.<br>*Always emit fmov when lowering copy.<br><br>Please reivew, thanks.<br></div><div><a href="http://llvm-reviews.chandlerc.com/D1671" target="_blank" style="font-family:arial,sans-serif;font-size:14px">http://llvm-reviews.chandlerc.com/D1671</a><br>
</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2013/9/13 Tim Northover <span dir="ltr"><<a href="mailto:t.p.northover@gmail.com" target="_blank">t.p.northover@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Kevin,<br>
<br>
> I see many developers use <a href="http://llvm-reviews.chandlerc.com" target="_blank">llvm-reviews.chandlerc.com</a> for review, so I upload my patch there.<br>
<br>
Good idea, I quite like the software when you get used to it. One<br>
thing: it's best to put llvm-commits in the CC list, otherwise most<br>
people will have no idea the patch is there.<br>
<br>
A few more comments:<br>
<br>
> setOperationAction(ISD::EXTRACT_VECTOR_ELT, MVT::v8i8, Custom);<br>
<br>
I'm hoping these won't be necessary any more (in fact they're the main<br>
reason I decided to implement the RegisterOperand change when I did) .<br>
Now that VPR64 and VPR128 are more sanely related, you should be able<br>
to write patterns for this instead of the custom lowering.<br>
<br>
>  BuildMI(MBB, I, DL, get(AArch64::INSsw), DestReg)<br>
>          .addReg(DestReg)<br>
<br>
I still think the separate NEON instructions are a mistake. Last time<br>
you asked for solid evidence, and I obviously couldn't say much. I've<br>
run a set of tight loops exercising the product { NEON, Scalar, Mixed}<br>
x { FMOV, INS }.<br>
<br>
As expected, mixing NEON and scalar instructions made essentially no<br>
difference but using INS instead of FMOV slowed the program down<br>
(massively, until I decided to be kind and break the artificial<br>
register-dependency introduced, then only slightly).<br>
<br>
> multiclass Neon_SMOV_pattern2 <RegisterOperand OpVPR, ValueType OpTy,<br>
>                               Operand OpImm, Instruction SMOVI> {<br>
 >def : Pat<(i64 (sext<br>
<br>
There's no need for a multiclass with just one member. You can use a<br>
class instead.<br>
<br>
Cheers.<br>
<span class="HOEnZb"><font color="#888888"><br>
Tim.<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Best Regards,<div><br></div><div>Kevin Qin</div></div>
</div>