Correct the sort logic in AsmMatcherEmmitter.cpp

[XinWang10]

Summary

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.

[skan]

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

5

Put this in a test for 16-bit mode. Otherwise we would lose test coverage.

[barannikov88]

llvm/test/MC/X86/pr22028.s

17

Is this correct? 65536 does not fit into a 16-bit word.

[skan]

llvm/test/MC/X86/pr22028.s

17

It aligns with the GAS's behavior.

bash$ cat 1.s

.code16
push 65536

bash$ as 1.s -o 1.o
bash$ objdump -M i8086 -d 1.o

0000000000000000 <.text>:
   0:   ff 36 00 00             pushw  0x0

[XinWang10]

• add test for call in 16 bit

[skan]

Can you make the summary easier to read? Developers have to check the line number of the source file to understand it, if you mention the logic by line 633

[skan]

LGTM

llvm/lib/Target/X86/X86InstrArithmetic.td

def X86sub_flag_nocf : PatFrag<(ops node:$lhs, node:$rhs),
                                (X86sub_flag node:$lhs, node:$rhs), [{
   // Only use DEC if the result is used.
   return !SDValue(N, 0).use_empty() && hasNoCarryFlagUses(SDValue(N, 1));
 }]>;
 
 let Defs = [EFLAGS] in {
 let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
 let isConvertibleToThreeAddress = 1, CodeSize = 2 in { // Can xform into LEA.
-def INC8r  : INCDECR<MRM0r, "inc", Xi8, X86add_flag_nocf>;
-def INC16r : INCDECR<MRM0r, "inc", Xi16, X86add_flag_nocf>;
-def INC32r : INCDECR<MRM0r, "inc", Xi32, X86add_flag_nocf>;
-def INC64r : INCDECR<MRM0r, "inc", Xi64, X86add_flag_nocf>;
-} // isConvertibleToThreeAddress = 1, CodeSize = 2
-
 // Short forms only valid in 32-bit mode. Selected during MCInst lowering.
 let CodeSize = 1, hasSideEffects = 0 in {
 def INC16r_alt : INCDECR_ALT<0x40, "inc", Xi16>;
 def INC32r_alt : INCDECR_ALT<0x40, "inc", Xi32>;
 } // CodeSize = 1, hasSideEffects = 0
+
+def INC8r  : INCDECR<MRM0r, "inc", Xi8, X86add_flag_nocf>;
+def INC16r : INCDECR<MRM0r, "inc", Xi16, X86add_flag_nocf>;
+def INC32r : INCDECR<MRM0r, "inc", Xi32, X86add_flag_nocf>;
+def INC64r : INCDECR<MRM0r, "inc", Xi64, X86add_flag_nocf>;
+} // isConvertibleToThreeAddress = 1, CodeSize = 2
 } // Constraints = "$src1 = $dst", SchedRW
 
 let CodeSize = 2, SchedRW = [WriteALURMW] in {
 let Predicates = [UseIncDec] in {
   def INC8m  : INCDECM<MRM0m, "inc", Xi8, 1>;
   def INC16m : INCDECM<MRM0m, "inc", Xi16, 1>;
   def INC32m : INCDECM<MRM0m, "inc", Xi32, 1>;
 } // Predicates
 let Predicates = [UseIncDec, In64BitMode] in {
   def INC64m : INCDECM<MRM0m, "inc", Xi64, 1>;
 } // Predicates
 } // CodeSize = 2, SchedRW
 
 let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
+// Short forms only valid in 32-bit mode. Selected during MCInst lowering.
+let CodeSize = 1, hasSideEffects = 0 in {
+def DEC16r_alt : INCDECR_ALT<0x48, "dec", Xi16>;
+def DEC32r_alt : INCDECR_ALT<0x48, "dec", Xi32>;
+} // CodeSize = 1, hasSideEffects = 0
+
 let isConvertibleToThreeAddress = 1, CodeSize = 2 in { // Can xform into LEA.
 def DEC8r  : INCDECR<MRM1r, "dec", Xi8, X86sub_flag_nocf>;
 def DEC16r : INCDECR<MRM1r, "dec", Xi16, X86sub_flag_nocf>;
 def DEC32r : INCDECR<MRM1r, "dec", Xi32, X86sub_flag_nocf>;
 def DEC64r : INCDECR<MRM1r, "dec", Xi64, X86sub_flag_nocf>;
 } // isConvertibleToThreeAddress = 1, CodeSize = 2
-
-// Short forms only valid in 32-bit mode. Selected during MCInst lowering.
-let CodeSize = 1, hasSideEffects = 0 in {
-def DEC16r_alt : INCDECR_ALT<0x48, "dec", Xi16>;
-def DEC32r_alt : INCDECR_ALT<0x48, "dec", Xi32>;
-} // CodeSize = 1, hasSideEffects = 0
 } // Constraints = "$src1 = $dst", SchedRW
 
 let CodeSize = 2, SchedRW = [WriteALURMW] in {
 let Predicates = [UseIncDec] in {
   def DEC8m  : INCDECM<MRM1m, "dec", Xi8, -1>;
   def DEC16m : INCDECM<MRM1m, "dec", Xi16, -1>;
   def DEC32m : INCDECM<MRM1m, "dec", Xi32, -1>;
 } // Predicates

llvm/lib/Target/X86/X86InstrControl.td

let Uses = [ESP, SSP] in {
     def CALLpcrel32 : Ii32PCRel<0xE8, RawFrm,
                            (outs), (ins i32imm_brtarget:$dst),
                            "call{l}\t$dst", []>, OpSize32,
                       Requires<[Not64BitMode]>, Sched<[WriteJump]>;
     let hasSideEffects = 0 in
       def CALLpcrel16 : Ii16PCRel<0xE8, RawFrm,
                              (outs), (ins i16imm_brtarget:$dst),
                              "call{w}\t$dst", []>, OpSize16,
-                        Sched<[WriteJump]>;
+                        Requires<[Not64BitMode]>, Sched<[WriteJump]>;
     def CALL16r     : I<0xFF, MRM2r, (outs), (ins GR16:$dst),
                         "call{w}\t{*}$dst", [(X86call GR16:$dst)]>,
                       OpSize16, Requires<[Not64BitMode]>, Sched<[WriteJump]>;
     def CALL16m     : I<0xFF, MRM2m, (outs), (ins i16mem:$dst),
                         "call{w}\t{*}$dst", [(X86call (loadi16 addr:$dst))]>,
                         OpSize16, Requires<[Not64BitMode,FavorMemIndirectCall]>,
                         Sched<[WriteJumpLd]>;
     def CALL32r     : I<0xFF, MRM2r, (outs), (ins GR32:$dst),

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

 0x67, 0x48 0xa1 0x5a 0x5a 0x5a 0x5a
 
 # CHECK: movabsq %rax, 6510615555426900570
 0x48 0xa3 0x5a 0x5a 0x5a 0x5a 0x5a 0x5a 0x5a 0x5a
 
 # CHECK: movq %rax, 1515870810
 0x67, 0x48 0xa3 0x5a 0x5a 0x5a 0x5a
 
-# CHECK: callw 32767
-0x66 0xe8 0xff 0x7f
+# CHECK: callq 32767
+0xe8,0xff,0x7f,0x00,0x00
 
 # TODO: Should display data16 prefixes.
 # CHECK-NOT: data16
 # CHECK: callq 32767
 0x66 0x66 0x48 0xe8 0xff 0x7f 0x00 0x00
 
 # CHECK: jmp -32769
 0xe9 0xff 0x7f 0xff 0xff

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

 // CHECK: andw (%rdx), %r14w 
 // CHECK: encoding: [0x66,0x44,0x23,0x32]        
 andw (%rdx), %r14w 
 
 // CHECK: callq 64 
 // CHECK: encoding: [0xe8,A,A,A,A]         
 callq 64 
 
-// CHECK: callw 64 
-// CHECK: encoding: [0x66,0xe8,A,A]         
-callw 64 
-
 // CHECK: cbtw 
 // CHECK: encoding: [0x66,0x98]          
 cbtw 
 
 // CHECK: cwtl 
 // CHECK: encoding: [0x98]          
 cwtl 
 

llvm/test/MC/X86/pr22028.s

 // RUN: llvm-mc -triple i386-unknown-unknown-code16 -show-encoding %s | FileCheck --check-prefix=CHECK16 %s
 // RUN: llvm-mc -triple i386-unknown-unknown -show-encoding %s | FileCheck %s
 // RUN: llvm-mc -triple i686-unknown-unknown -show-encoding %s | FileCheck %s 
   
 .intel_syntax
   
 push 0
 push -1
 push 30
 push 257
 push 65536
 
 //CHECK16:  pushw $0                      # encoding: [0x6a,0x00]
 //CHECK16:  pushw $-1                 # encoding: [0x6a,0xff]
 //CHECK16:  pushw $30                     # encoding: [0x6a,0x1e]
 //CHECK16:  pushw $257                    # encoding: [0x68,0x01,0x01]
-//CHECK16:  pushl $65536                  # encoding: [0x66,0x68,0x00,0x00,0x01,0x00]
+//CHECK16:  pushw $65536                  # encoding: [0x68,0x00,0x00]
 
 //CHECK:  pushl $0                      # encoding: [0x6a,0x00]
 //CHECK:  pushl $-1                     # encoding: [0x6a,0xff]
 //CHECK:  pushl $30                     # encoding: [0x6a,0x1e]
 //CHECK:  pushl $257                    # encoding: [0x68,0x01,0x01,0x00,0x00]
 //CHECK:  pushl $65536                  # encoding: [0x68,0x00,0x00,0x01,0x00]

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

 call  $0x7ace,$0x7ace
 lcall $0x7ace,$0x7ace
 jmp $0x7ace,$0x7ace
 ljmp  $0x7ace,$0x7ace
 
 // CHECK: calll a
 // CHECK: calll a
 // CHECK: calll a
+// CHECK: callw 42
+// CHECK: encoding: [0xe8,A,A]
  calll a
 data32 call a
 data32 callw a
+callw 42
 
 // CHECK:      ljmpl $1, $2
 // CHECK-NEXT: ljmpl $1, $2
 data32 ljmp $1, $2
 data32 ljmpw $1, $2
 
 // CHECK: incb  %al # encoding: [0xfe,0xc0]
   incb %al

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

 // RUN: llvm-mc -triple x86_64-unknown-unknown --show-encoding %s | FileCheck %s
 
-// PR7195
-// CHECK: callw 42
-// CHECK: encoding: [0x66,0xe8,A,A]
-       callw 42
-
 // rdar://8127102
 // CHECK: movq  %gs:(%rdi), %rax
 // CHECK: encoding: [0x65,0x48,0x8b,0x07]
 movq  %gs:(%rdi), %rax
 
 // CHECK: crc32b  %bl, %eax
 // CHECK:  encoding: [0xf2,0x0f,0x38,0xf0,0xc3]
         crc32b  %bl, %eax

llvm/utils/TableGen/AsmMatcherEmitter.cpp

bool operator<(const MatchableInfo &RHS) const {
     // orderings wouldn't be ambiguous using \see couldMatchAmbiguouslyWith().
     for (unsigned i = 0, e = AsmOperands.size(); i != e; ++i) {
       if (*AsmOperands[i].Class < *RHS.AsmOperands[i].Class)
         return true;
       if (*RHS.AsmOperands[i].Class < *AsmOperands[i].Class)
         return false;
     }
 
-    // Give matches that require more features higher precedence. This is useful
-    // because we cannot define AssemblerPredicates with the negation of
-    // processor features. For example, ARM v6 "nop" may be either a HINT or
-    // MOV. With v6, we want to match HINT. The assembler has no way to
-    // predicate MOV under "NoV6", but HINT will always match first because it
-    // requires V6 while MOV does not.
-    if (RequiredFeatures.size() != RHS.RequiredFeatures.size())
-      return RequiredFeatures.size() > RHS.RequiredFeatures.size();
-
     // For X86 AVX/AVX512 instructions, we prefer vex encoding because the
     // vex encoding size is smaller. Since X86InstrSSE.td is included ahead
     // of X86InstrAVX512.td, the AVX instruction ID is less than AVX512 ID.
     // We use the ID to sort AVX instruction before AVX512 instruction in
     // matching table.
     if (TheDef->isSubClassOf("Instruction") &&
         TheDef->getValueAsBit("HasPositionOrder"))
       return TheDef->getID() < RHS.TheDef->getID();
 
+    // Give matches that require more features higher precedence. This is useful
+    // because we cannot define AssemblerPredicates with the negation of
+    // processor features. For example, ARM v6 "nop" may be either a HINT or
+    // MOV. With v6, we want to match HINT. The assembler has no way to
+    // predicate MOV under "NoV6", but HINT will always match first because it
+    // requires V6 while MOV does not.
+    if (RequiredFeatures.size() != RHS.RequiredFeatures.size())
+      return RequiredFeatures.size() > RHS.RequiredFeatures.size();
+
     return false;
   }
 
   /// couldMatchAmbiguouslyWith - Check whether this matchable could
   /// ambiguously match the same set of operands as \p RHS (without being a
   /// strictly superior match).
   bool couldMatchAmbiguouslyWith(const MatchableInfo &RHS) const {
     // The primary comparator is the instruction mnemonic.