[rfc/patch][pr 22995] Swapped register order of "cmp" instructions
Craig Topper
craig.topper at gmail.com
Mon Mar 30 16:50:06 PDT 2015
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>
More information about the llvm-commits
mailing list