[X86][AsmParser] Omit predicate In64BitMode for instructions w/ GP64 operand in X86InstrArithmetic.td, NFCI

[skan]

Summary

As we discussed in D138639, we usually omit In64BitMode on instructions that
have a GR64 operand b/c

1. Assembler would never parse a GR64 register name outside of 64-bit mode
2. GR64/i64 is not legal during ISEL outside of 64-bit mode
3. Adding them might increase the size of the table in X86GenDAGISel.inc

For instructions that do not have an explicit GR64 operand, they're assumed to have an implicit In64BitMode predicate when W bit is set in AsmParser.

Since no test fails w/o the change in AsmParser, I add a test in this patch.

I will do similar clean in other files if this patch is accepted.

[pengfei]

I think we cannot remove it for memory only variants. The addr is legal on both 32-bits and 64-bits.

[skan]

In D149833#4318229, @pengfei wrote:

I think we cannot remove it for memory only variants. The addr is legal on both 32-bits and 64-bits.

But looking at the definition of loadi64

def loadi64  : PatFrag<(ops node:$ptr), (i64 (load node:$ptr))>;

, which is the operator of addr, it seems loadi64 is only matched in 64-bit mode due to i64.

[pengfei]

In D149833#4318345, @skan wrote:

In D149833#4318229, @pengfei wrote:

I think we cannot remove it for memory only variants. The addr is legal on both 32-bits and 64-bits.

But looking at the definition of loadi64

def loadi64  : PatFrag<(ops node:$ptr), (i64 (load node:$ptr))>;

, which is the operator of addr, it seems loadi64 is only matched in 64-bit mode due to i64.

That makes sense to me. Although I still have concerns about that, I cannot find a evidence for now. So LGTM.

[craig.topper]

None of these instructions have GPR operands to preventing parsing.

llvm/lib/Target/X86/X86InstrArithmetic.td

602

This instruction doesn't have a GPR operand. It's just incq (%rax) If the pointer doesn't have any register or uses 32 bit registers what prevents it from parsing?

[skan]

Check W bit in X86AmsParser

[skan]

llvm/lib/Target/X86/X86InstrArithmetic.td

602

This instruction doesn't have a GPR operand. It's just incq (%rax) If the pointer doesn't have any register or uses 32 bit registers what prevents it from parsing?

@craig.topper We can check if W bit is set.

[skan]

Remove redundant blank in test

[craig.topper]

llvm/lib/Target/X86/X86InstrArithmetic.td

602

Why create a new way to do this when we already have predicates?

[craig.topper]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

4193 ↗

(On Diff #520059)

The issue also appears on the VEX and EVEX encoded memory form of vextractps.

[skan]

llvm/lib/Target/X86/X86InstrArithmetic.td

602

I intended to simplify the code in TD. But I agreed that it would make the code in AsmParser look worse. I will close this.

[pengfei]

llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp

4193 ↗

(On Diff #520059)

Should !(TSFlags & X86II::EncodingMask) be used to prevent for VEX and EVEX? Anyway, I agree it's a bit big hammer for this.

llvm/lib/Target/X86/X86InstrArithmetic.td

 
 // 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
 } // Constraints = "$src1 = $dst", SchedRW
 
-let CodeSize = 2, SchedRW = [WriteALURMW] in {
-let Predicates = [UseIncDec] in {
+let CodeSize = 2, SchedRW = [WriteALURMW], 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
+} // CodeSize = 2, SchedRW, Predicates
 
 let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
 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 {
+let CodeSize = 2, SchedRW = [WriteALURMW], 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
-let Predicates = [UseIncDec, In64BitMode] in {
   def DEC64m : INCDECM<MRM1m, "dec", Xi64, -1>;
-} // Predicates
-} // CodeSize = 2, SchedRW
+} // CodeSize = 2, SchedRW, Predicates
 } // Defs = [EFLAGS]
 
 // Extra precision multiplication
 
 // AL is really implied by AX, but the registers in Defs must match the
 // SDNode results (i8, i32).
 // AL,AH = AL*GR8
 let Defs = [AL,EFLAGS,AX], Uses = [AL] in

 let mayLoad = 1, hasSideEffects = 0 in {
 let Defs = [AX,DX,EFLAGS], Uses = [AX] in
 def MUL16m : MulOpM<0xF7, MRM4m, "mul", Xi16, WriteIMul16, []>;
 // EAX,EDX = EAX*[mem32]
 let Defs = [EAX,EDX,EFLAGS], Uses = [EAX] in
 def MUL32m : MulOpM<0xF7, MRM4m, "mul", Xi32, WriteIMul32, []>;
 // RAX,RDX = RAX*[mem64]
 let Defs = [RAX,RDX,EFLAGS], Uses = [RAX] in
-def MUL64m : MulOpM<0xF7, MRM4m, "mul", Xi64, WriteIMul64, []>,
-                Requires<[In64BitMode]>;
+def MUL64m : MulOpM<0xF7, MRM4m, "mul", Xi64, WriteIMul64, []>;
 }
 
 let hasSideEffects = 0 in {
 // AL,AH = AL*GR8
 let Defs = [AL,EFLAGS,AX], Uses = [AL] in
 def IMUL8r  : MulOpR<0xF6, MRM5r, "imul", Xi8, WriteIMul8, []>;
 // AX,DX = AX*GR16
 let Defs = [AX,DX,EFLAGS], Uses = [AX] in

 // AX,DX = AX*[mem16]
 let Defs = [AX,DX,EFLAGS], Uses = [AX] in
 def IMUL16m : MulOpM<0xF7, MRM5m, "imul", Xi16, WriteIMul16, []>;
 // EAX,EDX = EAX*[mem32]
 let Defs = [EAX,EDX,EFLAGS], Uses = [EAX] in
 def IMUL32m : MulOpM<0xF7, MRM5m, "imul", Xi32, WriteIMul32, []>;
 // RAX,RDX = RAX*[mem64]
 let Defs = [RAX,RDX,EFLAGS], Uses = [RAX] in
-def IMUL64m : MulOpM<0xF7, MRM5m, "imul", Xi64, WriteIMul64, []>,
-                 Requires<[In64BitMode]>;
+def IMUL64m : MulOpM<0xF7, MRM5m, "imul", Xi64, WriteIMul64, []>;
 }
 
 let Defs = [EFLAGS] in {
 let Constraints = "$src1 = $dst" in {
 let isCommutable = 1 in {
 // X = IMUL Y, Z --> X = IMUL Z, Y
 // Register-Register Signed Integer Multiply
 def IMUL16rr : IMulOpRR<0xAF, "imul", Xi16, WriteIMul16Reg>;

 def DIV8m  : MulOpM<0xF6, MRM6m, "div", Xi8, WriteDiv8, []>;
 let Defs = [AX,DX,EFLAGS], Uses = [AX,DX] in
 // DX:AX/[mem16] = AX,DX
 def DIV16m : MulOpM<0xF7, MRM6m, "div", Xi16, WriteDiv16, []>;
 let Defs = [EAX,EDX,EFLAGS], Uses = [EAX,EDX] in    // EDX:EAX/[mem32] = EAX,EDX
 def DIV32m : MulOpM<0xF7, MRM6m, "div", Xi32, WriteDiv32, []>;
 // RDX:RAX/[mem64] = RAX,RDX
 let Defs = [RAX,RDX,EFLAGS], Uses = [RAX,RDX] in
-def DIV64m : MulOpM<0xF7, MRM6m, "div", Xi64, WriteDiv64, []>,
-             Requires<[In64BitMode]>;
+def DIV64m : MulOpM<0xF7, MRM6m, "div", Xi64, WriteDiv64, []>;
 }
 
 // Signed division/remainder.
 let Defs = [AL,AH,EFLAGS], Uses = [AX] in
 // AX/r8 = AL,AH
 def IDIV8r : MulOpR<0xF6, MRM7r, "idiv", Xi8, WriteIDiv8, []>;
 let Defs = [AX,DX,EFLAGS], Uses = [AX,DX] in
 // DX:AX/r16 = AX,DX

 let Defs = [AX,DX,EFLAGS], Uses = [AX,DX] in
 // DX:AX/[mem16] = AX,DX
 def IDIV16m: MulOpM<0xF7, MRM7m, "idiv", Xi16, WriteIDiv16, []>;
 let Defs = [EAX,EDX,EFLAGS], Uses = [EAX,EDX] in
 // EDX:EAX/[mem32] = EAX,EDX
 def IDIV32m: MulOpM<0xF7, MRM7m, "idiv", Xi32, WriteIDiv32, []>;
 let Defs = [RAX,RDX,EFLAGS], Uses = [RAX,RDX] in // RDX:RAX/[mem64] = RAX,RDX
 // RDX:RAX/[mem64] = RAX,RDX
-def IDIV64m: MulOpM<0xF7, MRM7m, "idiv", Xi64, WriteIDiv64, []>,
-             Requires<[In64BitMode]>;
+def IDIV64m: MulOpM<0xF7, MRM7m, "idiv", Xi64, WriteIDiv64, []>;
 }
 } // hasSideEffects = 0
 
 //===----------------------------------------------------------------------===//
 //  Two address Instructions.
 //
 
 // unary instructions
 let CodeSize = 2 in {
 let Defs = [EFLAGS] in {
 let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
 def NEG8r  : NegOpR<0xF6, "neg", Xi8>;
 def NEG16r : NegOpR<0xF7, "neg", Xi16>;
 def NEG32r : NegOpR<0xF7, "neg", Xi32>;
 def NEG64r : NegOpR<0xF7, "neg", Xi64>;
 } // Constraints = "$src1 = $dst", SchedRW
 
 // Read-modify-write negate.
 let SchedRW = [WriteALURMW] in {
 def NEG8m  : NegOpM<0xF6, "neg", Xi8>;
 def NEG16m : NegOpM<0xF7, "neg", Xi16>;
 def NEG32m : NegOpM<0xF7, "neg", Xi32>;
-def NEG64m : NegOpM<0xF7, "neg", Xi64>, Requires<[In64BitMode]>;
+def NEG64m : NegOpM<0xF7, "neg", Xi64>;
 } // SchedRW
 } // Defs = [EFLAGS]
 
 
 // Note: NOT does not set EFLAGS!
 
 let Constraints = "$src1 = $dst", SchedRW = [WriteALU] in {
 def NOT8r  : NotOpR<0xF6, "not", Xi8>;
 def NOT16r : NotOpR<0xF7, "not", Xi16>;
 def NOT32r : NotOpR<0xF7, "not", Xi32>;
 def NOT64r : NotOpR<0xF7, "not", Xi64>;
 } // Constraints = "$src1 = $dst", SchedRW
 
 let SchedRW = [WriteALURMW] in {
 def NOT8m  : NotOpM<0xF6, "not", Xi8>;
 def NOT16m : NotOpM<0xF7, "not", Xi16>;
 def NOT32m : NotOpM<0xF7, "not", Xi32>;
-def NOT64m : NotOpM<0xF7, "not", Xi64>, Requires<[In64BitMode]>;
+def NOT64m : NotOpM<0xF7, "not", Xi64>;
 } // SchedRW
 } // CodeSize
 
 /// ArithBinOp_RF - This is an arithmetic binary operator where the pattern is
 /// defined with "(set GPR:$dst, EFLAGS, (...".
 ///
 /// It would be nice to get rid of the second and third argument here, but
 /// tblgen can't handle dependent type references aggressively enough: PR8330
let mayLoad = 1, mayStore = 1 in {
       def NAME#32mr   : BinOpMR_RMW<BaseOpc, mnemonic, Xi32, opnode>;
       def NAME#64mr   : BinOpMR_RMW<BaseOpc, mnemonic, Xi64, opnode>;
     }
 
     // NOTE: These are order specific, we want the mi8 forms to be listed
     // first so that they are slightly preferred to the mi forms.
     def NAME#16mi8  : BinOpMI8_RMW<mnemonic, Xi16, opnode, MemMRM>;
     def NAME#32mi8  : BinOpMI8_RMW<mnemonic, Xi32, opnode, MemMRM>;
-    let Predicates = [In64BitMode] in
     def NAME#64mi8  : BinOpMI8_RMW<mnemonic, Xi64, opnode, MemMRM>;
 
     def NAME#8mi    : BinOpMI_RMW<0x80, mnemonic, Xi8 , opnode, MemMRM>;
     def NAME#16mi   : BinOpMI_RMW<0x80, mnemonic, Xi16, opnode, MemMRM>;
     def NAME#32mi   : BinOpMI_RMW<0x80, mnemonic, Xi32, opnode, MemMRM>;
-    let Predicates = [In64BitMode] in
     def NAME#64mi32 : BinOpMI_RMW<0x80, mnemonic, Xi64, opnode, MemMRM>;
 
     // These are for the disassembler since 0x82 opcode behaves like 0x80, but
     // not in 64-bit mode.
     let Predicates = [Not64BitMode], isCodeGenOnly = 1, ForceDisassemble = 1,
         hasSideEffects = 0 in {
       let Constraints = "$src1 = $dst" in
         def NAME#8ri8 : BinOpRI8_RF<0x82, mnemonic, Xi8, null_frag, RegMRM>;
let Uses = [EFLAGS], Defs = [EFLAGS] in {
     def NAME#16mr   : BinOpMR_RMW_FF<BaseOpc, mnemonic, Xi16, opnode>;
     def NAME#32mr   : BinOpMR_RMW_FF<BaseOpc, mnemonic, Xi32, opnode>;
     def NAME#64mr   : BinOpMR_RMW_FF<BaseOpc, mnemonic, Xi64, opnode>;
 
     // NOTE: These are order specific, we want the mi8 forms to be listed
     // first so that they are slightly preferred to the mi forms.
     def NAME#16mi8  : BinOpMI8_RMW_FF<mnemonic, Xi16, opnode, MemMRM>;
     def NAME#32mi8  : BinOpMI8_RMW_FF<mnemonic, Xi32, opnode, MemMRM>;
-    let Predicates = [In64BitMode] in
     def NAME#64mi8  : BinOpMI8_RMW_FF<mnemonic, Xi64, opnode, MemMRM>;
 
     def NAME#8mi    : BinOpMI_RMW_FF<0x80, mnemonic, Xi8 , opnode, MemMRM>;
     def NAME#16mi   : BinOpMI_RMW_FF<0x80, mnemonic, Xi16, opnode, MemMRM>;
     def NAME#32mi   : BinOpMI_RMW_FF<0x80, mnemonic, Xi32, opnode, MemMRM>;
-    let Predicates = [In64BitMode] in
     def NAME#64mi32 : BinOpMI_RMW_FF<0x80, mnemonic, Xi64, opnode, MemMRM>;
 
     // These are for the disassembler since 0x82 opcode behaves like 0x80, but
     // not in 64-bit mode.
     let Predicates = [Not64BitMode], isCodeGenOnly = 1, ForceDisassemble = 1,
         hasSideEffects = 0 in {
       let Constraints = "$src1 = $dst" in
         def NAME#8ri8 : BinOpRI8_RFF<0x82, mnemonic, Xi8, null_frag, RegMRM>;
let Defs = [EFLAGS] in {
     def NAME#16mr   : BinOpMR_F<BaseOpc, mnemonic, Xi16, opnode>;
     def NAME#32mr   : BinOpMR_F<BaseOpc, mnemonic, Xi32, opnode>;
     def NAME#64mr   : BinOpMR_F<BaseOpc, mnemonic, Xi64, opnode>;
 
     // NOTE: These are order specific, we want the mi8 forms to be listed
     // first so that they are slightly preferred to the mi forms.
     def NAME#16mi8  : BinOpMI8_F<mnemonic, Xi16, opnode, MemMRM>;
     def NAME#32mi8  : BinOpMI8_F<mnemonic, Xi32, opnode, MemMRM>;
-    let Predicates = [In64BitMode] in
     def NAME#64mi8  : BinOpMI8_F<mnemonic, Xi64, opnode, MemMRM>;
 
     def NAME#8mi    : BinOpMI_F<0x80, mnemonic, Xi8 , opnode, MemMRM>;
     def NAME#16mi   : BinOpMI_F<0x80, mnemonic, Xi16, opnode, MemMRM>;
     def NAME#32mi   : BinOpMI_F<0x80, mnemonic, Xi32, opnode, MemMRM>;
-    let Predicates = [In64BitMode] in
     def NAME#64mi32 : BinOpMI_F<0x80, mnemonic, Xi64, opnode, MemMRM>;
 
     // These are for the disassembler since 0x82 opcode behaves like 0x80, but
     // not in 64-bit mode.
     let Predicates = [Not64BitMode], isCodeGenOnly = 1, ForceDisassemble = 1,
         hasSideEffects = 0 in {
       def NAME#8ri8 : BinOpRI8_F<0x82, mnemonic, Xi8, null_frag, RegMRM>;
       let mayLoad = 1 in
let Defs = [EFLAGS] in {
     def TEST8ri    : BinOpRI_F<0xF6, "test", Xi8 , X86testpat, MRM0r>;
     def TEST16ri   : BinOpRI_F<0xF6, "test", Xi16, X86testpat, MRM0r>;
     def TEST32ri   : BinOpRI_F<0xF6, "test", Xi32, X86testpat, MRM0r>;
     def TEST64ri32 : BinOpRI_F<0xF6, "test", Xi64, X86testpat, MRM0r>;
 
     def TEST8mi    : BinOpMI_F<0xF6, "test", Xi8 , X86testpat, MRM0m>;
     def TEST16mi   : BinOpMI_F<0xF6, "test", Xi16, X86testpat, MRM0m>;
     def TEST32mi   : BinOpMI_F<0xF6, "test", Xi32, X86testpat, MRM0m>;
-    let Predicates = [In64BitMode] in
     def TEST64mi32 : BinOpMI_F<0xF6, "test", Xi64, X86testpat, MRM0m>;
   } // Defs = [EFLAGS]
 
   def TEST8i8    : BinOpAI_F<0xA8, "test", Xi8 , AL,
                              "{$src, %al|al, $src}">;
   def TEST16i16  : BinOpAI_F<0xA8, "test", Xi16, AX,
                              "{$src, %ax|ax, $src}">;
   def TEST32i32  : BinOpAI_F<0xA8, "test", Xi32, EAX,