[llvm] r177011 - Teach X86 MC instruction lowering that VMOVAPSrr and other VEX-encoded register to register moves should be switched from using the MRMSrcReg form to the MRMDestReg form if the source register is a 64-bit extended register and the destination register is not. This allows the instruction to be encoded using the 2-byte VEX form instead of the 3-byte VEX form. The GNU assembler has similar behavior.

Michael Liao michael.liao at intel.com
Fri Mar 15 20:49:13 PDT 2013


Cool. hope that buildbot goes green. - michael

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





More information about the llvm-commits mailing list