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

[Rafael Espíndola]

Ah, much better, thanks!

An updated patch is attached.

On 30 March 2015 at 19:50, Craig Topper <craig.topper at gmail.com> wrote:
> 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 --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: text/x-patch
Size: 2773 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150330/e5863175/attachment.bin>