Committed in r177221 with some comment additions and a better test name.<br><br><div class="gmail_quote">On Fri, Mar 15, 2013 at 8:08 PM, Michael Liao <span dir="ltr"><<a href="mailto:michael.liao@intel.com" target="_blank">michael.liao@intel.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Here is the full patch including old JIT code emitter. In addition,<br>
logic in MC lowering is revised. VMOVSS/SD needs check operand 0 and 2<br>
and the use of _REV is only enabled it's profitable.<br>
<br>
Yours<br>
<span class="HOEnZb"><font color="#888888">- Michael<br>
</font></span><div class="im HOEnZb"><br>
On Fri, 2013-03-15 at 19:18 -0700, Craig Topper wrote:<br>
</div><div class="HOEnZb"><div class="h5">> This doesn't look right. Shouldn't that be getOperand(CurOp)?<br>
><br>
> if (X86II::isX86_64ExtendedReg(MI.getOperand(1).getReg()))<br>
> VEX_R = 0x0;<br>
> + CurOp++;<br>
><br>
><br>
> I'll go ahead and finish this up. I'll also fix the old JIT code<br>
> emitter.<br>
><br>
> On Fri, Mar 15, 2013 at 6:43 PM, Michael Liao <<a href="mailto:michael.liao@intel.com">michael.liao@intel.com</a>><br>
> wrote:<br>
> Here is the revised patch. VMOVSS/VMOVSD should check operand<br>
> 0 and 2<br>
> and operand 1 is encoded in VEX.vvvv.<br>
><br>
> Yours<br>
> - Michael<br>
><br>
> On Fri, 2013-03-15 at 18:32 -0700, Michael Liao wrote:<br>
> > Hi Craig<br>
> ><br>
> > It's turned out that VEX.vvvv is not supported in DstReg<br>
> form. I add<br>
> > quick fix to that. It won't crash during code emission on<br>
> VMOVSS. Test<br>
> > case is added as well. Please have a look.<br>
> ><br>
> > Yours<br>
> > - Michael<br>
> ><br>
> > On Fri, 2013-03-15 at 15:38 -0700, Craig Topper wrote:<br>
> > > What assertion is failing?<br>
> > ><br>
> > > On Fri, Mar 15, 2013 at 2:31 PM, Nadav Rotem<br>
> <<a href="mailto:nrotem@apple.com">nrotem@apple.com</a>> wrote:<br>
> > > Hi Craig,<br>
> > ><br>
> > ><br>
> > > This commit causes an assertion failure in the X86<br>
> code<br>
> > > emitter and one of the build bots is failing. I<br>
> attached a<br>
> > > test case.<br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > > Thanks,<br>
> > > - Nadav<br>
> > ><br>
> > ><br>
> > ><br>
> > > On Mar 14, 2013, at 12:09 AM, Craig Topper<br>
> > > <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br>
> > ><br>
> > > > Author: ctopper<br>
> > > > Date: Thu Mar 14 02:09:57 2013<br>
> > > > New Revision: 177011<br>
> > > ><br>
> > > > URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project?rev=177011&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=177011&view=rev</a><br>
> > > > Log:<br>
> > > > Teach X86 MC instruction lowering that VMOVAPSrr<br>
> and other<br>
> > > > VEX-encoded register to register moves should be<br>
> switched<br>
> > > > from using the MRMSrcReg form to the MRMDestReg<br>
> form if the<br>
> > > > source register is a 64-bit extended register<br>
> and the<br>
> > > > destination register is not. This allows the<br>
> instruction to<br>
> > > > be encoded using the 2-byte VEX form instead of<br>
> the 3-byte<br>
> > > > VEX form. The GNU assembler has similar<br>
> behavior.<br>
> > > ><br>
> > > > Modified:<br>
> > > > llvm/trunk/lib/Target/X86/X86MCInstLower.cpp<br>
> > > ><br>
> > > > Modified:<br>
> llvm/trunk/lib/Target/X86/X86MCInstLower.cpp<br>
> > > > URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86MCInstLower.cpp?rev=177011&r1=177010&r2=177011&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86MCInstLower.cpp?rev=177011&r1=177010&r2=177011&view=diff</a><br>
> > > ><br>
> ==============================================================================<br>
> > > > --- llvm/trunk/lib/Target/X86/X86MCInstLower.cpp<br>
> (original)<br>
> > > > +++ llvm/trunk/lib/Target/X86/X86MCInstLower.cpp<br>
> Thu Mar 14<br>
> > > > 02:09:57 2013<br>
> > > > @@ -407,6 +407,48 @@ ReSimplify:<br>
> > > > LowerUnaryToTwoAddr(OutMI, X86::XOR32rr); //<br>
> MOV32r0 -><br>
> > > > XOR32rr<br>
> > > > break;<br>
> > > ><br>
> > > > + // Commute operands to get a smaller encoding<br>
> by using<br>
> > > > VEX.R instead of VEX.B<br>
> > > > + // if one of the registers is extended, but<br>
> other isn't.<br>
> > > > + case X86::VMOVAPDrr:<br>
> > > > + case X86::VMOVAPDYrr:<br>
> > > > + case X86::VMOVAPSrr:<br>
> > > > + case X86::VMOVAPSYrr:<br>
> > > > + case X86::VMOVDQArr:<br>
> > > > + case X86::VMOVDQAYrr:<br>
> > > > + case X86::VMOVDQUrr:<br>
> > > > + case X86::VMOVDQUYrr:<br>
> > > > + case X86::VMOVSDrr:<br>
> > > > + case X86::VMOVSSrr:<br>
> > > > + case X86::VMOVUPDrr:<br>
> > > > + case X86::VMOVUPDYrr:<br>
> > > > + case X86::VMOVUPSrr:<br>
> > > > + case X86::VMOVUPSYrr: {<br>
> > > > + if<br>
> > > ><br>
> (X86II::isX86_64ExtendedReg(OutMI.getOperand(0).getReg()) &&<br>
> > > > + !<br>
> > > ><br>
> X86II::isX86_64ExtendedReg(OutMI.getOperand(1).getReg()))<br>
> > > > + break;<br>
> > > > +<br>
> > > > + unsigned NewOpc;<br>
> > > > + switch (OutMI.getOpcode()) {<br>
> > > > + default: llvm_unreachable("Invalid<br>
> opcode");<br>
> > > > + case X86::VMOVAPDrr: NewOpc =<br>
> X86::VMOVAPDrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVAPDYrr: NewOpc =<br>
> X86::VMOVAPDYrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVAPSrr: NewOpc =<br>
> X86::VMOVAPSrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVAPSYrr: NewOpc =<br>
> X86::VMOVAPSYrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVDQArr: NewOpc =<br>
> X86::VMOVDQArr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVDQAYrr: NewOpc =<br>
> X86::VMOVDQAYrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVDQUrr: NewOpc =<br>
> X86::VMOVDQUrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVDQUYrr: NewOpc =<br>
> X86::VMOVDQUYrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVSDrr: NewOpc =<br>
> X86::VMOVSDrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVSSrr: NewOpc =<br>
> X86::VMOVSSrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVUPDrr: NewOpc =<br>
> X86::VMOVUPDrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVUPDYrr: NewOpc =<br>
> X86::VMOVUPDYrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVUPSrr: NewOpc =<br>
> X86::VMOVUPSrr_REV;<br>
> > > > break;<br>
> > > > + case X86::VMOVUPSYrr: NewOpc =<br>
> X86::VMOVUPSYrr_REV;<br>
> > > > break;<br>
> > > > + }<br>
> > > > + OutMI.setOpcode(NewOpc);<br>
> > > > + break;<br>
> > > > + }<br>
> > > > +<br>
> > > > // TAILJMPr64, CALL64r, CALL64pcrel32 - These<br>
> instructions<br>
> > > > have register<br>
> > > > // inputs modeled as normal uses instead of<br>
> implicit<br>
> > > > uses. As such, truncate<br>
> > > > // off all but the first operand (the<br>
> callee). FIXME:<br>
> > > > Change isel.<br>
> > > ><br>
> > > ><br>
> > > > _______________________________________________<br>
> > > > llvm-commits mailing list<br>
> > > > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> > > ><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > ><br>
> > > --<br>
> > > ~Craig<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><br>
> ><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><br>
><br>
><br>
><br>
><br>
><br>
> --<br>
> ~Craig<br>
<br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>~Craig