Skip to content

Commit 9a24ba2

Browse files
committed
Correct the sort logic in AsmMatcherEmmitter.cpp
The logic from line 633 to 640 is specific for ARM as the comments said, it will make all the targets will prefer to using instruction with more predicates when compiler do AsmMatching. And for code from line 642 to 649, X86 want to use the order records written in source file to sort the instructions. So X86 could be affected by this logic. (These code could be arrived only by X86) After change this, seems AVX instructions have not be affected but it exposed some other errors for instruction push and call. CALLpcrel16 could not be used in 64 bit mode, we need add Predicate for it. And for push instruction, previously because pushi32 has predicates = [Not64bitmode], so it precede pushi16, which is incorrect here, we should get pushw here and it also align with gcc. Reviewed By: skan Differential Revision: https://reviews.llvm.org/D150436
1 parent e74bb25 commit 9a24ba2

File tree

8 files changed

+28
-34
lines changed

8 files changed

+28
-34
lines changed

llvm/lib/Target/X86/X86InstrArithmetic.td

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -578,17 +578,17 @@ def X86sub_flag_nocf : PatFrag<(ops node:$lhs, node:$rhs),
578578
let Defs = [EFLAGS] in {
579579
let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
580580
let isConvertibleToThreeAddress = 1, CodeSize = 2 in { // Can xform into LEA.
581-
def INC8r : INCDECR<MRM0r, "inc", Xi8, X86add_flag_nocf>;
582-
def INC16r : INCDECR<MRM0r, "inc", Xi16, X86add_flag_nocf>;
583-
def INC32r : INCDECR<MRM0r, "inc", Xi32, X86add_flag_nocf>;
584-
def INC64r : INCDECR<MRM0r, "inc", Xi64, X86add_flag_nocf>;
585-
} // isConvertibleToThreeAddress = 1, CodeSize = 2
586-
587581
// Short forms only valid in 32-bit mode. Selected during MCInst lowering.
588582
let CodeSize = 1, hasSideEffects = 0 in {
589583
def INC16r_alt : INCDECR_ALT<0x40, "inc", Xi16>;
590584
def INC32r_alt : INCDECR_ALT<0x40, "inc", Xi32>;
591585
} // CodeSize = 1, hasSideEffects = 0
586+
587+
def INC8r : INCDECR<MRM0r, "inc", Xi8, X86add_flag_nocf>;
588+
def INC16r : INCDECR<MRM0r, "inc", Xi16, X86add_flag_nocf>;
589+
def INC32r : INCDECR<MRM0r, "inc", Xi32, X86add_flag_nocf>;
590+
def INC64r : INCDECR<MRM0r, "inc", Xi64, X86add_flag_nocf>;
591+
} // isConvertibleToThreeAddress = 1, CodeSize = 2
592592
} // Constraints = "$src1 = $dst", SchedRW
593593

594594
let CodeSize = 2, SchedRW = [WriteALURMW] in {
@@ -603,18 +603,18 @@ let Predicates = [UseIncDec, In64BitMode] in {
603603
} // CodeSize = 2, SchedRW
604604

605605
let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
606+
// Short forms only valid in 32-bit mode. Selected during MCInst lowering.
607+
let CodeSize = 1, hasSideEffects = 0 in {
608+
def DEC16r_alt : INCDECR_ALT<0x48, "dec", Xi16>;
609+
def DEC32r_alt : INCDECR_ALT<0x48, "dec", Xi32>;
610+
} // CodeSize = 1, hasSideEffects = 0
611+
606612
let isConvertibleToThreeAddress = 1, CodeSize = 2 in { // Can xform into LEA.
607613
def DEC8r : INCDECR<MRM1r, "dec", Xi8, X86sub_flag_nocf>;
608614
def DEC16r : INCDECR<MRM1r, "dec", Xi16, X86sub_flag_nocf>;
609615
def DEC32r : INCDECR<MRM1r, "dec", Xi32, X86sub_flag_nocf>;
610616
def DEC64r : INCDECR<MRM1r, "dec", Xi64, X86sub_flag_nocf>;
611617
} // isConvertibleToThreeAddress = 1, CodeSize = 2
612-
613-
// Short forms only valid in 32-bit mode. Selected during MCInst lowering.
614-
let CodeSize = 1, hasSideEffects = 0 in {
615-
def DEC16r_alt : INCDECR_ALT<0x48, "dec", Xi16>;
616-
def DEC32r_alt : INCDECR_ALT<0x48, "dec", Xi32>;
617-
} // CodeSize = 1, hasSideEffects = 0
618618
} // Constraints = "$src1 = $dst", SchedRW
619619

620620
let CodeSize = 2, SchedRW = [WriteALURMW] in {

llvm/lib/Target/X86/X86InstrControl.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ let isCall = 1 in
212212
def CALLpcrel16 : Ii16PCRel<0xE8, RawFrm,
213213
(outs), (ins i16imm_brtarget:$dst),
214214
"call{w}\t$dst", []>, OpSize16,
215-
Sched<[WriteJump]>;
215+
Requires<[Not64BitMode]>, Sched<[WriteJump]>;
216216
def CALL16r : I<0xFF, MRM2r, (outs), (ins GR16:$dst),
217217
"call{w}\t{*}$dst", [(X86call GR16:$dst)]>,
218218
OpSize16, Requires<[Not64BitMode]>, Sched<[WriteJump]>;

llvm/test/MC/Disassembler/X86/x86-64.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,8 @@
326326
# CHECK: movq %rax, 1515870810
327327
0x67, 0x48 0xa3 0x5a 0x5a 0x5a 0x5a
328328

329-
# CHECK: callw 32767
330-
0x66 0xe8 0xff 0x7f
329+
# CHECK: callq 32767
330+
0xe8,0xff,0x7f,0x00,0x00
331331

332332
# TODO: Should display data16 prefixes.
333333
# CHECK-NOT: data16

llvm/test/MC/X86/I86-64.s

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -676,10 +676,6 @@ andw (%rdx), %r14w
676676
// CHECK: encoding: [0xe8,A,A,A,A]
677677
callq 64
678678

679-
// CHECK: callw 64
680-
// CHECK: encoding: [0x66,0xe8,A,A]
681-
callw 64
682-
683679
// CHECK: cbtw
684680
// CHECK: encoding: [0x66,0x98]
685681
cbtw

llvm/test/MC/X86/pr22028.s

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ push 65536
1414
//CHECK16: pushw $-1 # encoding: [0x6a,0xff]
1515
//CHECK16: pushw $30 # encoding: [0x6a,0x1e]
1616
//CHECK16: pushw $257 # encoding: [0x68,0x01,0x01]
17-
//CHECK16: pushl $65536 # encoding: [0x66,0x68,0x00,0x00,0x01,0x00]
17+
//CHECK16: pushw $65536 # encoding: [0x68,0x00,0x00]
1818

1919
//CHECK: pushl $0 # encoding: [0x6a,0x00]
2020
//CHECK: pushl $-1 # encoding: [0x6a,0xff]

llvm/test/MC/X86/x86-16.s

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,9 +549,12 @@ ljmp $0x7ace,$0x7ace
549549
// CHECK: calll a
550550
// CHECK: calll a
551551
// CHECK: calll a
552+
// CHECK: callw 42
553+
// CHECK: encoding: [0xe8,A,A]
552554
calll a
553555
data32 call a
554556
data32 callw a
557+
callw 42
555558

556559
// CHECK: ljmpl $1, $2
557560
// CHECK-NEXT: ljmpl $1, $2

llvm/test/MC/X86/x86_64-encoding.s

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
// RUN: llvm-mc -triple x86_64-unknown-unknown --show-encoding %s | FileCheck %s
22

3-
// PR7195
4-
// CHECK: callw 42
5-
// CHECK: encoding: [0x66,0xe8,A,A]
6-
callw 42
7-
83
// rdar://8127102
94
// CHECK: movq %gs:(%rdi), %rax
105
// CHECK: encoding: [0x65,0x48,0x8b,0x07]

llvm/utils/TableGen/AsmMatcherEmitter.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -630,15 +630,6 @@ struct MatchableInfo {
630630
return false;
631631
}
632632

633-
// Give matches that require more features higher precedence. This is useful
634-
// because we cannot define AssemblerPredicates with the negation of
635-
// processor features. For example, ARM v6 "nop" may be either a HINT or
636-
// MOV. With v6, we want to match HINT. The assembler has no way to
637-
// predicate MOV under "NoV6", but HINT will always match first because it
638-
// requires V6 while MOV does not.
639-
if (RequiredFeatures.size() != RHS.RequiredFeatures.size())
640-
return RequiredFeatures.size() > RHS.RequiredFeatures.size();
641-
642633
// For X86 AVX/AVX512 instructions, we prefer vex encoding because the
643634
// vex encoding size is smaller. Since X86InstrSSE.td is included ahead
644635
// of X86InstrAVX512.td, the AVX instruction ID is less than AVX512 ID.
@@ -648,6 +639,15 @@ struct MatchableInfo {
648639
TheDef->getValueAsBit("HasPositionOrder"))
649640
return TheDef->getID() < RHS.TheDef->getID();
650641

642+
// Give matches that require more features higher precedence. This is useful
643+
// because we cannot define AssemblerPredicates with the negation of
644+
// processor features. For example, ARM v6 "nop" may be either a HINT or
645+
// MOV. With v6, we want to match HINT. The assembler has no way to
646+
// predicate MOV under "NoV6", but HINT will always match first because it
647+
// requires V6 while MOV does not.
648+
if (RequiredFeatures.size() != RHS.RequiredFeatures.size())
649+
return RequiredFeatures.size() > RHS.RequiredFeatures.size();
650+
651651
return false;
652652
}
653653

0 commit comments

Comments
 (0)