Skip to content

Commit

Permalink
[X86] Change register&memory TEST instructions from MRMSrcMem to MRMD…
Browse files Browse the repository at this point in the history
…stMem

Summary:
Intel documentation shows the memory operand as the first operand. But we currently treat it as the second operand. Conceptually the order doesn't matter since it doesn't write memory. We have aliases to parse with the operands in either order and the isel matching is commutable.

For the register&register form order does matter for the assembly parser. PR22995 was previously filed and fixed by changing the register&register form from MRMSrcReg to MRMDestReg to match gas. Ideally the memory form should match by using MRMDestMem.

I believe this supercedes D38025 which was trying to switch the register&register form back to pre-PR22995.

Reviewers: aymanmus, RKSimon, zvi

Reviewed By: aymanmus

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D38120

llvm-svn: 314639
  • Loading branch information
topperc committed Oct 1, 2017
1 parent 0023060 commit c20b46d
Show file tree
Hide file tree
Showing 15 changed files with 52 additions and 53 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
Expand Up @@ -380,7 +380,7 @@ void X86MCCodeEmitter::emitMemModRMByte(const MCInst &MI, unsigned Op,
return X86::reloc_riprel_4byte_movq_load;
case X86::CALL64m:
case X86::JMP64m:
case X86::TEST64rm:
case X86::TEST64mr:
case X86::ADC64rm:
case X86::ADD64rm:
case X86::AND64rm:
Expand Down
21 changes: 10 additions & 11 deletions llvm/lib/Target/X86/X86InstrArithmetic.td
Expand Up @@ -652,21 +652,20 @@ class ITy<bits<8> opcode, Format f, X86TypeInfo typeinfo, dag outs, dag ins,

// BinOpRR - Instructions like "add reg, reg, reg".
class BinOpRR<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,
dag outlist, list<dag> pattern, InstrItinClass itin,
Format f = MRMDestReg>
: ITy<opcode, f, typeinfo, outlist,
dag outlist, list<dag> pattern, InstrItinClass itin>
: ITy<opcode, MRMDestReg, typeinfo, outlist,
(ins typeinfo.RegClass:$src1, typeinfo.RegClass:$src2),
mnemonic, "{$src2, $src1|$src1, $src2}", pattern, itin>,
Sched<[WriteALU]>;

// BinOpRR_F - Instructions like "cmp reg, Reg", where the pattern has
// just a EFLAGS as a result.
class BinOpRR_F<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,
SDPatternOperator opnode, Format f = MRMDestReg>
SDPatternOperator opnode>
: BinOpRR<opcode, mnemonic, typeinfo, (outs),
[(set EFLAGS,
(opnode typeinfo.RegClass:$src1, typeinfo.RegClass:$src2))],
IIC_BIN_NONMEM, f>;
IIC_BIN_NONMEM>;

// BinOpRR_RF - Instructions like "add reg, reg, reg", where the pattern has
// both a regclass and EFLAGS as a result.
Expand Down Expand Up @@ -727,7 +726,7 @@ class BinOpRM<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,

// BinOpRM_F - Instructions like "cmp reg, [mem]".
class BinOpRM_F<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,
SDPatternOperator opnode>
SDNode opnode>
: BinOpRM<opcode, mnemonic, typeinfo, (outs),
[(set EFLAGS,
(opnode typeinfo.RegClass:$src1, (typeinfo.LoadNode addr:$src2)))]>;
Expand Down Expand Up @@ -837,7 +836,7 @@ class BinOpMR_RMW_FF<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,

// BinOpMR_F - Instructions like "cmp [mem], reg".
class BinOpMR_F<bits<8> opcode, string mnemonic, X86TypeInfo typeinfo,
SDNode opnode>
SDPatternOperator opnode>
: BinOpMR<opcode, mnemonic, typeinfo,
[(set EFLAGS, (opnode (load addr:$dst), typeinfo.RegClass:$src))]>;

Expand Down Expand Up @@ -1224,10 +1223,10 @@ let isCompare = 1 in {
def TEST64rr : BinOpRR_F<0x84, "test", Xi64, X86testpat>;
} // isCommutable

def TEST8rm : BinOpRM_F<0x84, "test", Xi8 , X86testpat>;
def TEST16rm : BinOpRM_F<0x84, "test", Xi16, X86testpat>;
def TEST32rm : BinOpRM_F<0x84, "test", Xi32, X86testpat>;
def TEST64rm : BinOpRM_F<0x84, "test", Xi64, X86testpat>;
def TEST8mr : BinOpMR_F<0x84, "test", Xi8 , X86testpat>;
def TEST16mr : BinOpMR_F<0x84, "test", Xi16, X86testpat>;
def TEST32mr : BinOpMR_F<0x84, "test", Xi32, X86testpat>;
def TEST64mr : BinOpMR_F<0x84, "test", Xi64, X86testpat>;

def TEST8ri : BinOpRI_F<0xF6, "test", Xi8 , X86testpat, MRM0r>;
def TEST16ri : BinOpRI_F<0xF6, "test", Xi16, X86testpat, MRM0r>;
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/Target/X86/X86InstrInfo.cpp
Expand Up @@ -375,9 +375,13 @@ X86InstrInfo::X86InstrInfo(X86Subtarget &STI)
{ X86::TAILJMPr64, X86::TAILJMPm64, TB_FOLDED_LOAD },
{ X86::TAILJMPr64_REX, X86::TAILJMPm64_REX, TB_FOLDED_LOAD },
{ X86::TEST16ri, X86::TEST16mi, TB_FOLDED_LOAD },
{ X86::TEST16rr, X86::TEST16mr, TB_FOLDED_LOAD },
{ X86::TEST32ri, X86::TEST32mi, TB_FOLDED_LOAD },
{ X86::TEST32rr, X86::TEST32mr, TB_FOLDED_LOAD },
{ X86::TEST64ri32, X86::TEST64mi32, TB_FOLDED_LOAD },
{ X86::TEST64rr, X86::TEST64mr, TB_FOLDED_LOAD },
{ X86::TEST8ri, X86::TEST8mi, TB_FOLDED_LOAD },
{ X86::TEST8rr, X86::TEST8mr, TB_FOLDED_LOAD },

// AVX 128-bit versions of foldable instructions
{ X86::VEXTRACTPSrr,X86::VEXTRACTPSmr, TB_FOLDED_STORE },
Expand Down Expand Up @@ -608,10 +612,6 @@ X86InstrInfo::X86InstrInfo(X86Subtarget &STI)
{ X86::SQRTSDr_Int, X86::SQRTSDm_Int, TB_NO_REVERSE },
{ X86::SQRTSSr, X86::SQRTSSm, 0 },
{ X86::SQRTSSr_Int, X86::SQRTSSm_Int, TB_NO_REVERSE },
{ X86::TEST16rr, X86::TEST16rm, 0 },
{ X86::TEST32rr, X86::TEST32rm, 0 },
{ X86::TEST64rr, X86::TEST64rm, 0 },
{ X86::TEST8rr, X86::TEST8rm, 0 },
// FIXME: TEST*rr EAX,EAX ---> CMP [mem], 0
{ X86::UCOMISDrr, X86::UCOMISDrm, 0 },
{ X86::UCOMISSrr, X86::UCOMISSrm, 0 },
Expand Down
16 changes: 8 additions & 8 deletions llvm/lib/Target/X86/X86InstrInfo.td
Expand Up @@ -3236,14 +3236,14 @@ defm : ShiftRotateByOneAlias<"ror", "ROR">;
FIXME */

// test: We accept "testX <reg>, <mem>" and "testX <mem>, <reg>" as synonyms.
def : InstAlias<"test{b}\t{$val, $mem|$mem, $val}",
(TEST8rm GR8 :$val, i8mem :$mem), 0>;
def : InstAlias<"test{w}\t{$val, $mem|$mem, $val}",
(TEST16rm GR16:$val, i16mem:$mem), 0>;
def : InstAlias<"test{l}\t{$val, $mem|$mem, $val}",
(TEST32rm GR32:$val, i32mem:$mem), 0>;
def : InstAlias<"test{q}\t{$val, $mem|$mem, $val}",
(TEST64rm GR64:$val, i64mem:$mem), 0>;
def : InstAlias<"test{b}\t{$mem, $val|$val, $mem}",
(TEST8mr i8mem :$mem, GR8 :$val), 0>;
def : InstAlias<"test{w}\t{$mem, $val|$val, $mem}",
(TEST16mr i16mem:$mem, GR16:$val), 0>;
def : InstAlias<"test{l}\t{$mem, $val|$val, $mem}",
(TEST32mr i32mem:$mem, GR32:$val), 0>;
def : InstAlias<"test{q}\t{$mem, $val|$val, $mem}",
(TEST64mr i64mem:$mem, GR64:$val), 0>;

// xchg: We accept "xchgX <reg>, <mem>" and "xchgX <mem>, <reg>" as synonyms.
def : InstAlias<"xchg{b}\t{$mem, $val|$val, $mem}",
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/Target/X86/X86MacroFusion.cpp
Expand Up @@ -82,10 +82,10 @@ static bool shouldScheduleAdjacent(const TargetInstrInfo &TII,
case X86::TEST32i32:
case X86::TEST64i32:
case X86::TEST64ri32:
case X86::TEST8rm:
case X86::TEST16rm:
case X86::TEST32rm:
case X86::TEST64rm:
case X86::TEST8mr:
case X86::TEST16mr:
case X86::TEST32mr:
case X86::TEST64mr:
case X86::TEST8ri_NOREX:
case X86::AND16i16:
case X86::AND16ri:
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/X86/X86SchedHaswell.td
Expand Up @@ -2099,9 +2099,9 @@ def: InstRW<[HWWriteResGroup18], (instregex "OR8rm")>;
def: InstRW<[HWWriteResGroup18], (instregex "POP(16|32|64)r(mr?)")>;
def: InstRW<[HWWriteResGroup18], (instregex "SUB(16|32|64)rm")>;
def: InstRW<[HWWriteResGroup18], (instregex "SUB8rm")>;
def: InstRW<[HWWriteResGroup18], (instregex "TEST(16|32|64)rm")>;
def: InstRW<[HWWriteResGroup18], (instregex "TEST(16|32|64)mr")>;
def: InstRW<[HWWriteResGroup18], (instregex "TEST8mi")>;
def: InstRW<[HWWriteResGroup18], (instregex "TEST8rm")>;
def: InstRW<[HWWriteResGroup18], (instregex "TEST8mr")>;
def: InstRW<[HWWriteResGroup18], (instregex "XOR(16|32|64)rm")>;
def: InstRW<[HWWriteResGroup18], (instregex "XOR8rm")>;

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/X86/X86SchedSandyBridge.td
Expand Up @@ -2010,9 +2010,9 @@ def: InstRW<[SBWriteResGroup70], (instregex "SUB(16|32|64)mi8")>;
def: InstRW<[SBWriteResGroup70], (instregex "SUB(16|32|64)mr")>;
def: InstRW<[SBWriteResGroup70], (instregex "SUB8mi")>;
def: InstRW<[SBWriteResGroup70], (instregex "SUB8mr")>;
def: InstRW<[SBWriteResGroup70], (instregex "TEST(16|32|64)rm")>;
def: InstRW<[SBWriteResGroup70], (instregex "TEST(16|32|64)mr")>;
def: InstRW<[SBWriteResGroup70], (instregex "TEST8mi")>;
def: InstRW<[SBWriteResGroup70], (instregex "TEST8rm")>;
def: InstRW<[SBWriteResGroup70], (instregex "TEST8mr")>;
def: InstRW<[SBWriteResGroup70], (instregex "XOR(16|32|64)mi8")>;
def: InstRW<[SBWriteResGroup70], (instregex "XOR(16|32|64)mr")>;
def: InstRW<[SBWriteResGroup70], (instregex "XOR8mi")>;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/X86/X86SchedSkylakeClient.td
Expand Up @@ -1634,9 +1634,9 @@ def: InstRW<[SKLWriteResGroup21], (instregex "OR8rm")>;
def: InstRW<[SKLWriteResGroup21], (instregex "POP(16|32|64)r(mr?)")>;
def: InstRW<[SKLWriteResGroup21], (instregex "SUB(16|32|64)rm")>;
def: InstRW<[SKLWriteResGroup21], (instregex "SUB8rm")>;
def: InstRW<[SKLWriteResGroup21], (instregex "TEST(16|32|64)rm")>;
def: InstRW<[SKLWriteResGroup21], (instregex "TEST(16|32|64)mr")>;
def: InstRW<[SKLWriteResGroup21], (instregex "TEST8mi")>;
def: InstRW<[SKLWriteResGroup21], (instregex "TEST8rm")>;
def: InstRW<[SKLWriteResGroup21], (instregex "TEST8mr")>;
def: InstRW<[SKLWriteResGroup21], (instregex "XOR(16|32|64)rm")>;
def: InstRW<[SKLWriteResGroup21], (instregex "XOR8rm")>;

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/X86/absolute-bit-mask.ll
Expand Up @@ -43,7 +43,7 @@ f:
define void @foo64(i64* %ptr) {
%load = load i64, i64* %ptr
; CHECK: movabsq $bit_mask64, %rax
; CHECK: testq (%rdi), %rax
; CHECK: testq %rax, (%rdi)
%and = and i64 %load, ptrtoint (i8* @bit_mask64 to i64)
%icmp = icmp eq i64 %and, 0
br i1 %icmp, label %t, label %f
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/X86/post-ra-sched-with-debug.mir
Expand Up @@ -292,7 +292,7 @@ body: |
%rcx = CMOVNE64rr killed %rcx, killed %rdx, implicit killed %eflags
%rcx = OR64rr killed %rcx, killed %rsi, implicit-def dead %eflags
%rdx = MOVSX64rm32 %rbx, 1, _, 0, _ :: (load 4, align 8)
TEST32rm killed %eax, killed %rcx, 4, killed %rdx, 0, _, implicit-def %eflags :: (load 4)
TEST32mr killed %rcx, 4, killed %rdx, 0, _, killed %eax, implicit-def %eflags :: (load 4)
JNE_1 %bb.2, implicit %eflags
JMP_1 %bb.3
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/X86/testl-commute.ll
Expand Up @@ -9,7 +9,7 @@ target triple = "x86_64-apple-darwin7"
define i32 @test(i32* %P, i32* %G) nounwind {
; CHECK-LABEL: test:
; CHECK-NOT: ret
; CHECK: testl (%{{.*}}), %{{.*}}
; CHECK: testl %{{.*}}, (%{{.*}})
; CHECK: ret

entry:
Expand All @@ -30,7 +30,7 @@ bb1: ; preds = %entry
define i32 @test2(i32* %P, i32* %G) nounwind {
; CHECK-LABEL: test2:
; CHECK-NOT: ret
; CHECK: testl (%{{.*}}), %{{.*}}
; CHECK: testl %{{.*}}, (%{{.*}})
; CHECK: ret

entry:
Expand All @@ -51,7 +51,7 @@ bb1: ; preds = %entry
define i32 @test3(i32* %P, i32* %G) nounwind {
; CHECK-LABEL: test3:
; CHECK-NOT: ret
; CHECK: testl (%{{.*}}), %{{.*}}
; CHECK: testl %{{.*}}, (%{{.*}})
; CHECK: ret

entry:
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/MC/Disassembler/X86/x86-16.txt
Expand Up @@ -318,10 +318,10 @@
# CHECK: sysretl
0x0f 0x07

# CHECK: testl -24(%ebp), %ecx
# CHECK: testl %ecx, -24(%ebp)
0x67 0x66 0x85 0x4d 0xe8

# CHECK: testl -24(%ebp), %ecx
# CHECK: testl %ecx, -24(%ebp)
0x67 0x66 0x85 0x4d 0xe8

# CHECK: pushw %cs
Expand Down
16 changes: 8 additions & 8 deletions llvm/test/MC/X86/intel-syntax.s
Expand Up @@ -532,14 +532,14 @@ xchg [ECX], EAX
xchg AX, [ECX]
xchg [ECX], AX

// CHECK: testq (%ecx), %rax
// CHECK: testq (%ecx), %rax
// CHECK: testl (%ecx), %eax
// CHECK: testl (%ecx), %eax
// CHECK: testw (%ecx), %ax
// CHECK: testw (%ecx), %ax
// CHECK: testb (%ecx), %al
// CHECK: testb (%ecx), %al
// CHECK: testq %rax, (%ecx)
// CHECK: testq %rax, (%ecx)
// CHECK: testl %eax, (%ecx)
// CHECK: testl %eax, (%ecx)
// CHECK: testw %ax, (%ecx)
// CHECK: testw %ax, (%ecx)
// CHECK: testb %al, (%ecx)
// CHECK: testb %al, (%ecx)
test RAX, [ECX]
test [ECX], RAX
test EAX, [ECX]
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/MC/X86/x86-16.s
Expand Up @@ -406,9 +406,9 @@ sysretl
// CHECK: encoding: [0x0f,0x07]

testl %ecx, -24(%ebp)
// CHECK: testl -24(%ebp), %ecx
// CHECK: testl %ecx, -24(%ebp)
testl -24(%ebp), %ecx
// CHECK: testl -24(%ebp), %ecx
// CHECK: testl %ecx, -24(%ebp)


push %cs
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/MC/X86/x86-32.s
Expand Up @@ -528,9 +528,9 @@ sysretl

// rdar://8018260
testl %ecx, -24(%ebp)
// CHECK: testl -24(%ebp), %ecx
// CHECK: testl %ecx, -24(%ebp)
testl -24(%ebp), %ecx
// CHECK: testl -24(%ebp), %ecx
// CHECK: testl %ecx, -24(%ebp)


// rdar://8407242
Expand Down

0 comments on commit c20b46d

Please sign in to comment.