[rfc/patch][pr 22995] Swapped register order of "cmp" instructions

[Craig Topper]

I agree these lines need the MRMSrcReg argument removed.

      def TEST8rr  : BinOpRR_F<0x84, "test", Xi8 , X86testpat, MRMSrcReg>;
      def TEST16rr : BinOpRR_F<0x84, "test", Xi16, X86testpat, MRMSrcReg>;
      def TEST32rr : BinOpRR_F<0x84, "test", Xi32, X86testpat, MRMSrcReg>;
      def TEST64rr : BinOpRR_F<0x84, "test", Xi64, X86testpat, MRMSrcReg>;

On Mon, Mar 30, 2015 at 3:31 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
wrote:

> I was talking about CMP, but the PR is about TEST only, right?
>
> If so, I think the right fix is to use the default MRMDestReg Format
> for TEST*rr, like we do for the other MR-encoded instructions.  Craig,
> does that sound right to you?
>
> On Mon, Mar 30, 2015 at 1:32 PM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
> >> I can't say anything about reasons, but this doesn't seem like a
> >> printing issue, rather one of default encoding?  There's 38/39 for "r/m,
> >> r", and 3A/3B for "r, r/m":  I guess MC and GAS don't agree on that.
> >
> > The encoding is different, but the instructions are symmetrical.
>
> Disregard, that doesn't apply to TEST, which only has one opcode.
> Sorry about the noise!
>
> Back to the actual issue:  I think MC is wrong. In the SDM, looking at
> the RM encoding of TEST:  it takes op1 = ModRM:r/m, and op2 =
> ModRM:reg.
>
> In AT&T syntax, for:
>
>   test %ebx, %eax
>
> The ModRM byte should be 0xd8 (2.1.5, table 2-2).   Feeding that to MC
> gives us 0xc3 instead:
>
>   testl %ebx, %eax              ## encoding: [0x85,0xc3]
>                                           ## <MCInst #2894 TEST32rr
>                                           ##  <MCOperand Reg:19>
>                                           ##  <MCOperand Reg:21>>
>
> However, disassembling via MC the MR encoding for CMP, which is 0x39
> 0xd8, we get:
>
>   cmpl %ebx, %eax              ## encoding: [0x39,0xd8]
>                                           ## <MCInst #540 CMP32rr
>                                           ##  <MCOperand Reg:19>
>                                           ##  <MCOperand Reg:21>>
>
> Tying it back to the .td, we defined TEST*rr with the MRMSrcReg
> format, and CMP with MRMDestReg, and this is the root problem.
> Grepping around for MRMSrcReg, we use it for the RM encoding: IMUL,
> ADCX, and the various "r, r/m" _REV instructions.  The MRMDestReg
> format is for the MR encoding, and should be used for TEST.
>
> -Ahmed
>



-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150330/49a1d766/attachment.html>