Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[X86][MC] Support encoding/decoding for APX variant ADD/SUB/ADC/SBB/OR/XOR/NEG/NOT instructions #76319

Merged
merged 10 commits into from
Dec 28, 2023

Conversation

KanRobert
Copy link
Contributor

@KanRobert KanRobert commented Dec 24, 2023

Four variants: promoted legacy, ND (new data destination), NF (no flags update) and NF_ND (NF + ND).

The syntax of NF instructions is aligned with GNU binutils.
https://sourceware.org/pipermail/binutils/2023-September/129545.html

@llvmbot llvmbot added backend:X86 mc Machine (object) code llvm:support labels Dec 24, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 24, 2023

@llvm/pr-subscribers-llvm-support

@llvm/pr-subscribers-backend-x86

Author: Shengchen Kan (KanRobert)

Changes

Four variants: promoted, ND, NF and NF_ND.


Patch is 37.51 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76319.diff

19 Files Affected:

  • (modified) llvm/include/llvm/Support/X86DisassemblerDecoderCommon.h (+8-1)
  • (modified) llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp (+7)
  • (modified) llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp (+5-1)
  • (modified) llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h (+1)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h (+4-1)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp (+3)
  • (modified) llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp (+22-2)
  • (modified) llvm/lib/Target/X86/X86InstrArithmetic.td (+73-23)
  • (modified) llvm/lib/Target/X86/X86InstrFormats.td (+2)
  • (modified) llvm/lib/Target/X86/X86InstrPredicates.td (+2)
  • (modified) llvm/lib/Target/X86/X86InstrUtils.td (+14)
  • (added) llvm/test/CodeGen/X86/apx/add.ll (+50)
  • (added) llvm/test/MC/Disassembler/X86/apx/add.txt (+66)
  • (added) llvm/test/MC/X86/apx/add-att.s (+53)
  • (added) llvm/test/MC/X86/apx/add-intel.s (+50)
  • (modified) llvm/utils/TableGen/X86DisassemblerTables.cpp (+19)
  • (modified) llvm/utils/TableGen/X86FoldTablesEmitter.cpp (+4-2)
  • (modified) llvm/utils/TableGen/X86RecognizableInstr.cpp (+24-4)
  • (modified) llvm/utils/TableGen/X86RecognizableInstr.h (+2)
diff --git a/llvm/include/llvm/Support/X86DisassemblerDecoderCommon.h b/llvm/include/llvm/Support/X86DisassemblerDecoderCommon.h
index b0683ac2e32c05..3aceb247a26c21 100644
--- a/llvm/include/llvm/Support/X86DisassemblerDecoderCommon.h
+++ b/llvm/include/llvm/Support/X86DisassemblerDecoderCommon.h
@@ -70,7 +70,8 @@ enum attributeBits {
   ATTR_EVEXKZ = 0x1 << 11,
   ATTR_EVEXB = 0x1 << 12,
   ATTR_REX2 = 0x1 << 13,
-  ATTR_max = 0x1 << 14,
+  ATTR_EVEXNF = 0x1 << 14,
+  ATTR_max = 0x1 << 15,
 };
 
 // Combinations of the above attributes that are relevant to instruction
@@ -137,12 +138,15 @@ enum attributeBits {
   ENUM_ENTRY(IC_VEX_L_W_XD, 5, "requires VEX, L, W and XD prefix")             \
   ENUM_ENTRY(IC_VEX_L_W_OPSIZE, 5, "requires VEX, L, W and OpSize")            \
   ENUM_ENTRY(IC_EVEX, 1, "requires an EVEX prefix")                            \
+  ENUM_ENTRY(IC_EVEX_NF, 2, "requires EVEX and NF prefix")                     \
   ENUM_ENTRY(IC_EVEX_XS, 2, "requires EVEX and the XS prefix")                 \
   ENUM_ENTRY(IC_EVEX_XD, 2, "requires EVEX and the XD prefix")                 \
   ENUM_ENTRY(IC_EVEX_OPSIZE, 2, "requires EVEX and the OpSize prefix")         \
+  ENUM_ENTRY(IC_EVEX_OPSIZE_NF, 3, "requires EVEX, NF and the OpSize prefix")  \
   ENUM_ENTRY(IC_EVEX_OPSIZE_ADSIZE, 3,                                         \
              "requires EVEX, OPSIZE and the ADSIZE prefix")                    \
   ENUM_ENTRY(IC_EVEX_W, 3, "requires EVEX and the W prefix")                   \
+  ENUM_ENTRY(IC_EVEX_W_NF, 4, "requires EVEX, W and NF prefix")                \
   ENUM_ENTRY(IC_EVEX_W_XS, 4, "requires EVEX, W, and XS prefix")               \
   ENUM_ENTRY(IC_EVEX_W_XD, 4, "requires EVEX, W, and XD prefix")               \
   ENUM_ENTRY(IC_EVEX_W_OPSIZE, 4, "requires EVEX, W, and OpSize")              \
@@ -187,10 +191,13 @@ enum attributeBits {
   ENUM_ENTRY(IC_EVEX_L2_W_XD_K, 4, "requires EVEX_K, L2, W and XD prefix")     \
   ENUM_ENTRY(IC_EVEX_L2_W_OPSIZE_K, 4, "requires EVEX_K, L2, W and OpSize")    \
   ENUM_ENTRY(IC_EVEX_B, 1, "requires an EVEX_B prefix")                        \
+  ENUM_ENTRY(IC_EVEX_B_NF, 2, "requires EVEX_NF and EVEX_B prefix")            \
   ENUM_ENTRY(IC_EVEX_XS_B, 2, "requires EVEX_B and the XS prefix")             \
   ENUM_ENTRY(IC_EVEX_XD_B, 2, "requires EVEX_B and the XD prefix")             \
   ENUM_ENTRY(IC_EVEX_OPSIZE_B, 2, "requires EVEX_B and the OpSize prefix")     \
+  ENUM_ENTRY(IC_EVEX_OPSIZE_B_NF, 3, "requires EVEX_B, NF and Opsize prefix")  \
   ENUM_ENTRY(IC_EVEX_W_B, 3, "requires EVEX_B and the W prefix")               \
+  ENUM_ENTRY(IC_EVEX_W_B_NF, 4, "requires EVEX_NF, EVEX_B and the W prefix")   \
   ENUM_ENTRY(IC_EVEX_W_XS_B, 4, "requires EVEX_B, W, and XS prefix")           \
   ENUM_ENTRY(IC_EVEX_W_XD_B, 4, "requires EVEX_B, W, and XD prefix")           \
   ENUM_ENTRY(IC_EVEX_W_OPSIZE_B, 4, "requires EVEX_B, W, and OpSize")          \
diff --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 1d40ce35c1b416..143da9c5e285c9 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -108,6 +108,8 @@ class X86AsmParser : public MCTargetAsmParser {
 
   // Does this instruction use apx extended register?
   bool UseApxExtendedReg = false;
+  // Is this instruction explicitly required not to update flags?
+  bool ForcedNoFlag = false;
 
 private:
   SMLoc consumeToken() {
@@ -3127,6 +3129,7 @@ bool X86AsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
   ForcedVEXEncoding = VEXEncoding_Default;
   ForcedDispEncoding = DispEncoding_Default;
   UseApxExtendedReg = false;
+  ForcedNoFlag = false;
 
   // Parse pseudo prefixes.
   while (true) {
@@ -3151,6 +3154,8 @@ bool X86AsmParser::ParseInstruction(ParseInstructionInfo &Info, StringRef Name,
         ForcedDispEncoding = DispEncoding_Disp8;
       else if (Prefix == "disp32")
         ForcedDispEncoding = DispEncoding_Disp32;
+      else if (Prefix == "nf")
+        ForcedNoFlag = true;
       else
         return Error(NameLoc, "unknown prefix");
 
@@ -3998,6 +4003,8 @@ unsigned X86AsmParser::checkTargetMatchPredicate(MCInst &Inst) {
 
   if (UseApxExtendedReg && !X86II::canUseApxExtendedReg(MCID))
     return Match_Unsupported;
+  if (ForcedNoFlag != static_cast<bool>(MCID.TSFlags & X86II::EVEX_NF))
+    return Match_Unsupported;
 
   if (ForcedVEXEncoding == VEXEncoding_EVEX &&
       (MCID.TSFlags & X86II::EncodingMask) != X86II::EVEX)
diff --git a/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp b/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
index 59e2008f56321b..347dc0d4ed43a7 100644
--- a/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
+++ b/llvm/lib/Target/X86/Disassembler/X86Disassembler.cpp
@@ -1169,7 +1169,11 @@ static int getInstructionID(struct InternalInstruction *insn,
         attrMask |= ATTR_EVEXKZ;
       if (bFromEVEX4of4(insn->vectorExtensionPrefix[3]))
         attrMask |= ATTR_EVEXB;
-      if (aaaFromEVEX4of4(insn->vectorExtensionPrefix[3]))
+      // nf bit is the MSB of aaa
+      if (nfFromEVEX4of4(insn->vectorExtensionPrefix[3]) &&
+          insn->opcodeType == MAP4)
+        attrMask |= ATTR_EVEXNF;
+      else if (aaaFromEVEX4of4(insn->vectorExtensionPrefix[3]))
         attrMask |= ATTR_EVEXK;
       if (lFromEVEX4of4(insn->vectorExtensionPrefix[3]))
         attrMask |= ATTR_VEXL;
diff --git a/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h b/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h
index decc45091941d7..4c7b1c094522eb 100644
--- a/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h
+++ b/llvm/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h
@@ -103,6 +103,7 @@ namespace X86Disassembler {
 #define bFromEVEX4of4(evex) bitFromOffset4(evex)
 #define v2FromEVEX4of4(evex) invertedBitFromOffset3(evex)
 #define aaaFromEVEX4of4(evex) threeBitsFromOffset0(evex)
+#define nfFromEVEX4of4(evex) bitFromOffset2(evex)
 
 // These enums represent Intel registers for use by the decoder.
 #define REGS_8BIT                                                              \
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h b/llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
index b0fcaef5f4b064..7c85697a021176 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
@@ -870,7 +870,10 @@ enum : uint64_t {
   ExplicitVEXPrefix = 2ULL << ExplicitOpPrefixShift,
   /// For instructions that are promoted to EVEX space for EGPR.
   ExplicitEVEXPrefix = 3ULL << ExplicitOpPrefixShift,
-  ExplicitOpPrefixMask = 3ULL << ExplicitOpPrefixShift
+  ExplicitOpPrefixMask = 3ULL << ExplicitOpPrefixShift,
+  /// EVEX_NF - Set if this instruction has EVEX.NF field set.
+  EVEX_NFShift = ExplicitOpPrefixShift + 2,
+  EVEX_NF = 1ULL << EVEX_NFShift
 };
 
 /// \returns true if the instruction with given opcode is a prefix.
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
index cab2f0a2e1c1a2..1947313a9dfb0b 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
@@ -369,6 +369,9 @@ void X86InstPrinterCommon::printInstFlags(const MCInst *MI, raw_ostream &O,
   else if (Flags & X86::IP_HAS_REPEAT)
     O << "\trep\t";
 
+  if (TSFlags & X86II::EVEX_NF)
+    O << "\t{nf}";
+
   // These all require a pseudo prefix
   if ((Flags & X86::IP_USE_VEX) ||
       (TSFlags & X86II::ExplicitOpPrefixMask) == X86II::ExplicitVEXPrefix)
diff --git a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
index 9e1f1eb97e7032..deb029d4a4074f 100644
--- a/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
+++ b/llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
@@ -251,6 +251,7 @@ class X86OpcodePrefixHelper {
   void setAAA(const MCInst &MI, unsigned OpNum) {
     EVEX_aaa = getRegEncoding(MI, OpNum);
   }
+  void setNF(bool V) { EVEX_aaa |= V << 2; }
 
   X86OpcodePrefixHelper(const MCRegisterInfo &MRI)
       : W(0), R(0), X(0), B(0), M(0), R2(0), X2(0), B2(0), VEX_4V(0), VEX_L(0),
@@ -987,6 +988,7 @@ X86MCCodeEmitter::emitVEXOpcodePrefix(int MemOperand, const MCInst &MI,
   }
 
   Prefix.setW(TSFlags & X86II::REX_W);
+  Prefix.setNF(TSFlags & X86II::EVEX_NF);
 
   bool HasEVEX_K = TSFlags & X86II::EVEX_K;
   bool HasVEX_4V = TSFlags & X86II::VEX_4V;
@@ -1049,6 +1051,9 @@ X86MCCodeEmitter::emitVEXOpcodePrefix(int MemOperand, const MCInst &MI,
 
   bool EncodeRC = false;
   uint8_t EVEX_rc = 0;
+  bool IsND = (TSFlags & X86II::OpMapMask) == X86II::T_MAP4 &&
+              (TSFlags & X86II::EVEX_B) && HasVEX_4V;
+
   unsigned CurOp = X86II::getOperandBias(Desc);
 
   switch (TSFlags & X86II::FormMask) {
@@ -1160,12 +1165,17 @@ X86MCCodeEmitter::emitVEXOpcodePrefix(int MemOperand, const MCInst &MI,
     //
     //  FMA4:
     //  dst(ModR/M.reg), src1(VEX_4V), src2(Imm[7:4]), src3(ModR/M),
+    //
+    //  NDD:
+    //  dst(VEX_4V), src1(ModR/M.reg), src2(ModR/M)
+    if (IsND)
+      Prefix.set4VV2(MI, CurOp++);
     Prefix.setRR2(MI, CurOp++);
 
     if (HasEVEX_K)
       Prefix.setAAA(MI, CurOp++);
 
-    if (HasVEX_4V)
+    if (!IsND && HasVEX_4V)
       Prefix.set4VV2(MI, CurOp++);
 
     Prefix.setBB2(MI, CurOp);
@@ -1209,6 +1219,11 @@ X86MCCodeEmitter::emitVEXOpcodePrefix(int MemOperand, const MCInst &MI,
     //  dst(ModR/M), src(ModR/M)
     //  dst(ModR/M), src(ModR/M), imm8
     //  dst(ModR/M), src1(VEX_4V), src2(ModR/M)
+    //
+    // NDD:
+    // dst(VEX_4V), src1(ModR/M), src2(ModR/M)
+    if (IsND)
+      Prefix.set4VV2(MI, CurOp++);
     Prefix.setBB2(MI, CurOp);
     Prefix.setX(MI, CurOp, 4);
     ++CurOp;
@@ -1216,7 +1231,7 @@ X86MCCodeEmitter::emitVEXOpcodePrefix(int MemOperand, const MCInst &MI,
     if (HasEVEX_K)
       Prefix.setAAA(MI, CurOp++);
 
-    if (HasVEX_4V)
+    if (!IsND && HasVEX_4V)
       Prefix.set4VV2(MI, CurOp++);
 
     Prefix.setRR2(MI, CurOp++);
@@ -1508,6 +1523,9 @@ void X86MCCodeEmitter::encodeInstruction(const MCInst &MI,
 
   unsigned OpcodeOffset = 0;
 
+  bool IsND = (TSFlags & X86II::OpMapMask) == X86II::T_MAP4 &&
+              (TSFlags & X86II::EVEX_B) && HasVEX_4V;
+
   uint64_t Form = TSFlags & X86II::FormMask;
   switch (Form) {
   default:
@@ -1576,6 +1594,8 @@ void X86MCCodeEmitter::encodeInstruction(const MCInst &MI,
 
     if (HasVEX_4V) // Skip 1st src (which is encoded in VEX_VVVV)
       ++SrcRegNum;
+    if (IsND) // Skip the NDD operand encoded in EVEX_VVVV
+      ++CurOp;
 
     emitRegModRMByte(MI.getOperand(CurOp),
                      getX86RegNum(MI.getOperand(SrcRegNum)), CB);
diff --git a/llvm/lib/Target/X86/X86InstrArithmetic.td b/llvm/lib/Target/X86/X86InstrArithmetic.td
index 22394545a7fa2b..9d2bc1da68d176 100644
--- a/llvm/lib/Target/X86/X86InstrArithmetic.td
+++ b/llvm/lib/Target/X86/X86InstrArithmetic.td
@@ -45,12 +45,12 @@ def PLEA64r   : PseudoI<(outs GR64:$dst), (ins anymem:$src), []>;
 }
 
 // BinOpRR - Instructions that read "reg, reg".
-class BinOpRR<bits<8> o, string m, X86TypeInfo t, dag out, list<dag> p>
+class BinOpRR<bits<8> o, string m, string args, X86TypeInfo t, dag out, list<dag> p>
   : ITy<o, MRMDestReg, t, out, (ins t.RegClass:$src1, t.RegClass:$src2), m,
-        "{$src2, $src1|$src1, $src2}", p>, Sched<[WriteALU]>;
+        args, p>, Sched<[WriteALU]>;
 // BinOpRR_F - Instructions that read "reg, reg" and write EFLAGS only.
 class BinOpRR_F<bits<8> o, string m, X86TypeInfo t, SDPatternOperator node>
-  : BinOpRR<o, m, t, (outs),
+  : BinOpRR<o, m, binop_args, t, (outs),
             [(set EFLAGS, (node t.RegClass:$src1, t.RegClass:$src2))]>,
     DefEFLAGS;
 // BinOpRR_F_Rev - Reversed encoding of BinOpRR_F
@@ -58,28 +58,38 @@ class BinOpRR_F_Rev<bits<8> o, string m, X86TypeInfo t>
   : BinOpRR_F<o, m, t, null_frag>, DisassembleOnly {
   let Form = MRMSrcReg;
 }
+// BinOpRR_R - Instructions that read "reg, reg" and write "reg".
+class BinOpRR_R<bits<8> o, string m, X86TypeInfo t, bit ndd = 0>
+  : BinOpRR<o, m, !if(!eq(ndd, 0), binop_args, binop_ndd_args), t,
+            (outs t.RegClass:$dst), []>, NDD<ndd>;
+// BinOpRR_R_Rev - Reversed encoding of BinOpRR_R
+class BinOpRR_R_Rev<bits<8> o, string m, X86TypeInfo t, bit ndd = 0>
+  : BinOpRR_R<o, m, t, ndd>, DisassembleOnly {
+  let Form = MRMSrcReg;
+}
 // BinOpRR_RF - Instructions that read "reg, reg", and write "reg", EFLAGS.
-class BinOpRR_RF<bits<8> o, string m, X86TypeInfo t, SDPatternOperator node>
-  : BinOpRR<o, m, t, (outs t.RegClass:$dst),
+class BinOpRR_RF<bits<8> o, string m, X86TypeInfo t, SDPatternOperator node, bit ndd = 0>
+  : BinOpRR<o, m, !if(!eq(ndd, 0), binop_args, binop_ndd_args), t,
+            (outs t.RegClass:$dst),
             [(set t.RegClass:$dst, EFLAGS,
-             (node t.RegClass:$src1, t.RegClass:$src2))]>, DefEFLAGS;
+             (node t.RegClass:$src1, t.RegClass:$src2))]>, DefEFLAGS, NDD<ndd>;
 // BinOpRR_RF_Rev - Reversed encoding of BinOpRR_RF.
-class BinOpRR_RF_Rev<bits<8> o, string m, X86TypeInfo t>
-  : BinOpRR_RF<o, m, t, null_frag>, DisassembleOnly {
+class BinOpRR_RF_Rev<bits<8> o, string m, X86TypeInfo t, bit ndd = 0>
+  : BinOpRR_RF<o, m, t, null_frag, ndd>, DisassembleOnly {
   let Form = MRMSrcReg;
 }
 // BinOpRRF_RF - Instructions that read "reg, reg", write "reg" and read/write
 // EFLAGS.
-class BinOpRRF_RF<bits<8> o, string m, X86TypeInfo t, SDPatternOperator node>
-  : BinOpRR<o, m, t, (outs t.RegClass:$dst),
+class BinOpRRF_RF<bits<8> o, string m, X86TypeInfo t, SDPatternOperator node, bit ndd = 0>
+  : BinOpRR<o, m, !if(!eq(ndd, 0), binop_args, binop_ndd_args), t, (outs t.RegClass:$dst),
             [(set t.RegClass:$dst, EFLAGS,
              (node t.RegClass:$src1, t.RegClass:$src2,
-             EFLAGS))]>, DefEFLAGS, UseEFLAGS {
+             EFLAGS))]>, DefEFLAGS, UseEFLAGS, NDD<ndd> {
   let SchedRW = [WriteADC];
 }
 // BinOpRRF_RF_Rev - Reversed encoding of BinOpRRF_RF
-class BinOpRRF_RF_Rev<bits<8> o, string m, X86TypeInfo t>
-  : BinOpRRF_RF<o, m, t, null_frag>, DisassembleOnly {
+class BinOpRRF_RF_Rev<bits<8> o, string m, X86TypeInfo t, bit ndd = 0>
+  : BinOpRRF_RF<o, m, t, null_frag, ndd>, DisassembleOnly {
   let Form = MRMSrcReg;
 }
 
@@ -640,20 +650,60 @@ multiclass ArithBinOp_RF<bits<8> BaseOpc, bits<8> BaseOpc2, bits<8> BaseOpc4,
                          SDNode opnodeflag, SDNode opnode,
                          bit CommutableRR, bit ConvertibleToThreeAddress,
                          bit ConvertibleToThreeAddressRR> {
-  let Constraints = "$src1 = $dst" in {
-    let isCommutable = CommutableRR,
-        isConvertibleToThreeAddress = ConvertibleToThreeAddressRR in {
+  let isCommutable = CommutableRR,
+      isConvertibleToThreeAddress = ConvertibleToThreeAddressRR in {
+    let Predicates = [NoNDD] in {
     def NAME#8rr  : BinOpRR_RF<BaseOpc, mnemonic, Xi8 , opnodeflag>;
     def NAME#16rr : BinOpRR_RF<BaseOpc, mnemonic, Xi16, opnodeflag>, OpSize16;
     def NAME#32rr : BinOpRR_RF<BaseOpc, mnemonic, Xi32, opnodeflag>, OpSize32;
     def NAME#64rr : BinOpRR_RF<BaseOpc, mnemonic, Xi64, opnodeflag>;
     }
+    let Predicates = [HasNDD, In64BitMode] in {
+    def NAME#8rr_ND  : BinOpRR_RF<BaseOpc, mnemonic, Xi8 , opnodeflag, 1>;
+    def NAME#16rr_ND : BinOpRR_RF<BaseOpc, mnemonic, Xi16, opnodeflag, 1>, PD;
+    def NAME#32rr_ND : BinOpRR_RF<BaseOpc, mnemonic, Xi32, opnodeflag, 1>;
+    def NAME#64rr_ND : BinOpRR_RF<BaseOpc, mnemonic, Xi64, opnodeflag, 1>;
+    def NAME#8rr_NF_ND  : BinOpRR_R<BaseOpc, mnemonic, Xi8, 1>, EVEX_NF;
+    def NAME#16rr_NF_ND : BinOpRR_R<BaseOpc, mnemonic, Xi16, 1>, EVEX_NF, PD;
+    def NAME#32rr_NF_ND : BinOpRR_R<BaseOpc, mnemonic, Xi32, 1>, EVEX_NF;
+    def NAME#64rr_NF_ND : BinOpRR_R<BaseOpc, mnemonic, Xi64, 1>, EVEX_NF;
+    }
+    let Predicates = [In64BitMode] in {
+    def NAME#8rr_NF  : BinOpRR_R<BaseOpc, mnemonic, Xi8>, T_MAP4, EVEX, EVEX_NF;
+    def NAME#16rr_NF : BinOpRR_R<BaseOpc, mnemonic, Xi16>, T_MAP4, EVEX, EVEX_NF, PD;
+    def NAME#32rr_NF : BinOpRR_R<BaseOpc, mnemonic, Xi32>, T_MAP4, EVEX, EVEX_NF;
+    def NAME#64rr_NF : BinOpRR_R<BaseOpc, mnemonic, Xi64>, T_MAP4, EVEX, EVEX_NF;
+    def NAME#8rr_EVEX  : BinOpRR_RF<BaseOpc, mnemonic, Xi8 , null_frag>, T_MAP4, EVEX, ExplicitEVEXPrefix;
+    def NAME#16rr_EVEX : BinOpRR_RF<BaseOpc, mnemonic, Xi16, null_frag>, T_MAP4, EVEX, PD, ExplicitEVEXPrefix;
+    def NAME#32rr_EVEX : BinOpRR_RF<BaseOpc, mnemonic, Xi32, null_frag>, T_MAP4, EVEX, ExplicitEVEXPrefix;
+    def NAME#64rr_EVEX : BinOpRR_RF<BaseOpc, mnemonic, Xi64, null_frag>, T_MAP4, EVEX, ExplicitEVEXPrefix;
+    }
+  }
 
   def NAME#8rr_REV  : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi8>;
   def NAME#16rr_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi16>, OpSize16;
   def NAME#32rr_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi32>, OpSize32;
   def NAME#64rr_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi64>;
+    let Predicates = [In64BitMode] in {
+    def NAME#8rr_EVEX_REV  : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi8>, T_MAP4, EVEX, ExplicitEVEXPrefix;
+    def NAME#16rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi16>, T_MAP4, EVEX, PD, ExplicitEVEXPrefix;
+    def NAME#32rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi32>, T_MAP4, EVEX, ExplicitEVEXPrefix;
+    def NAME#64rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi64>, T_MAP4, EVEX, ExplicitEVEXPrefix;
+    def NAME#8rr_ND_REV  : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi8, 1>;
+    def NAME#16rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi16, 1>, PD;
+    def NAME#32rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi32, 1>;
+    def NAME#64rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi64, 1>;
+    def NAME#8rr_NF_REV  : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi8>, T_MAP4, EVEX, EVEX_NF;
+    def NAME#16rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi16>, T_MAP4, EVEX, EVEX_NF, PD;
+    def NAME#32rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi32>, T_MAP4, EVEX, EVEX_NF;
+    def NAME#64rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi64>, T_MAP4, EVEX, EVEX_NF;
+    def NAME#8rr_NF_ND_REV  : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi8, 1>, EVEX_NF;
+    def NAME#16rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi16, 1>, EVEX_NF, PD;
+    def NAME#32rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi32, 1>, EVEX_NF;
+    def NAME#64rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi64, 1>, EVEX_NF;
+    }
 
+  let Constraints = "$src1 = $dst" in {
   def NAME#8rm   : BinOpRM_RF<BaseOpc2, mnemonic, Xi8 , opnodeflag>;
   def NAME#16rm  : BinOpRM_RF<BaseOpc2, mnemonic, Xi16, opnodeflag>, OpSize16;
   def NAME#32rm  : BinOpRM_RF<BaseOpc2, mnemonic, Xi32, opnodeflag>, OpSize32;
@@ -719,16 +769,16 @@ multiclass ArithBinOp_RFF<bits<8> BaseOpc, bits<8> BaseOpc2, bits<8> BaseOpc4,
                           string mnemonic, Format RegMRM, Format MemMRM,
                           SDNode opnode, bit CommutableRR,
                            bit ConvertibleToThreeAddress> {
-  let Constraints = "$src1 = $dst" in {
-    let isCommutable = CommutableRR in {
-    def NAME#8rr  : BinOpRRF_RF<BaseOpc, mnemonic, Xi8 , opnode>;
-      let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
-      def NAME#16rr : BinOpRRF_RF<BaseOpc, mnemonic, Xi16, opnode>, OpSize16;
-      def NAME#32rr : BinOpRRF_RF<BaseOpc, mnemonic, Xi32, opnode>, OpSize32;
-      def NAME#64rr : BinOpRRF_RF<BaseOpc, mnemonic, Xi64, opnode>;
+  let isCommutable = CommutableRR in {
+  def NAME#8rr  : BinOpRRF_RF<BaseOpc, mnemonic, Xi8 , opnode>;
+    let isConvertibleToThreeAddress = ConvertibleToThreeAddress in {
+    def NAME#16rr : BinOpRRF_RF<BaseOpc, mnemonic, Xi16, opnode>, OpSize16;
+    def NAME#32rr : BinOpRRF_RF<BaseOpc, mnemonic, Xi32, opnode>, OpSize32;
+    def NAME#64rr : BinOpRRF_RF<BaseOpc, mnemonic, Xi64, opnode>;
     } // isConvertibleToThreeAddress
   } // isCommutable
 
+  let Constraints = "$src1 = $dst" in {
   def NAME#8rr_REV  : BinOpRRF_RF_Rev<BaseOpc2, mnemonic, Xi8>;
   def NAME#16rr_REV : BinOpRRF_RF_Rev<BaseOpc2, mnemonic, Xi16>, OpSize16;
   def NAME#32rr_REV : BinOpRRF_RF_Rev<BaseOpc2, mnemonic, Xi32>, OpSize32;
diff --git a/llvm/lib/Target/X86/X86InstrFormats.td b/llvm/lib/Target/X86/X86InstrFormats.td
index 07e5576960d65c..6e76b44b66a307 100644
--- a/llvm/lib/Target/X86/X86InstrFormats.td
+++ b/llvm/lib/Target/X86/X86InstrF...
[truncated]

Copy link

github-actions bot commented Dec 24, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

KanRobert added a commit that referenced this pull request Dec 24, 2023
…n X86InstrArithmetic.td

This patch is to extract the NFC in #76319 into a separate commit.
@KanRobert KanRobert marked this pull request as draft December 24, 2023 11:14
KanRobert added a commit that referenced this pull request Dec 26, 2023
1. Remove these two classes b/c opcode is changed from 0xF6 to 0x66
   after promotion, then the classes become useless.
2. Remove `OpSize = OpSizeFixed` b/c the default value is OpSizeFixed.
3. Remove `let isCommutable = 1` b/c ADCX/ADOX is not VEX-encoding,
   we can not apply VEX3ToVEX2 optimization for it and the compiler
   never emits it.
4. Remove predicate `HasADX` due to no pattern

This patch is to extract the NFC in #76319 into a separate commit.
KanRobert added a commit that referenced this pull request Dec 26, 2023
This patch is to extract the NFC in #76319 into a separate commit.
@KanRobert KanRobert marked this pull request as ready for review December 26, 2023 14:30
@KanRobert KanRobert changed the title [X86][MC][CodeGen] Support encoding/decoding and pattern match for APX variant ADD instructions [X86][MC] Support encoding/decoding for APX variant ADD/SUB/ADC/SBB/OR/XOR instructions Dec 26, 2023
@KanRobert
Copy link
Contributor Author

This PR is ready for review.

KanRobert added a commit that referenced this pull request Dec 27, 2023
This patch is to extract the NFC in #76319 into a separate commit.
…R/XOR instructions

Four variants: promoted legacy, ND (new data destination), NF (no flags update) and NF_ND (NF + ND).
@@ -3996,6 +4001,8 @@ unsigned X86AsmParser::checkTargetMatchPredicate(MCInst &Inst) {

if (UseApxExtendedReg && !X86II::canUseApxExtendedReg(MCID))
return Match_Unsupported;
if (ForcedNoFlag != static_cast<bool>(MCID.TSFlags & X86II::EVEX_NF))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use !!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be ForcedNoFlag && (MCID.TSFlags & X86II::EVEX_NF)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will use !!. No, the logic is

  1. NF instruction can be matched only when {nf} exsits
  2. non-NF instruction can not be matched when {nf} exists

so the suggested code does not work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a not {nf} assembly can be matched to {nf} instruction? If you want to check for unexpected case, you can use an assert here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's b/c we don't add {nf} in the asm string in TD explicitly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then how can non {nf} instruction correctly matched to non {nf} instruction?
Or we should concat {nf} in asm string since its mandatory?

Copy link
Contributor Author

@KanRobert KanRobert Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NF instruction and non-NF instruction shares the same asm string in the TD file.
Fonr non {nf} asm, ForceNoFlag = false,
for NF instruction, (MCID.TSFlags & X86II::EVEX_NF) = true.

They're not equal. So non {nf} asm can only match non-NF instruction.

Or we should concat {nf} in asm string since its mandatory?

It has no benefits, complicates the code and not aligns with existing design like {evex}.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{evex} doesn't check in this way. Given a asm string without {nf}, it should be selected to non-NF instruction, otherwise, there's something wrong during match instruction. That's why I think it just need to assert for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. One usage of {evex}: ExplicitEVEX checks this way. Anyway, concat {nf} would complicate code in TD a lot.

llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h Outdated Show resolved Hide resolved
llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/X86/X86InstrArithmetic.td Outdated Show resolved Hide resolved
llvm/lib/Target/X86/X86InstrArithmetic.td Outdated Show resolved Hide resolved
llvm/utils/TableGen/X86RecognizableInstr.h Outdated Show resolved Hide resolved
llvm/test/MC/Disassembler/X86/apx/adc.txt Show resolved Hide resolved
Comment on lines 643 to 772
let Predicates = [In64BitMode] in {
def NAME#8rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi8>, PL;
def NAME#16rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi16>, PL, PD;
def NAME#32rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi32>, PL;
def NAME#64rr_EVEX_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi64>, PL;
def NAME#8rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi8, 1>;
def NAME#16rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi16, 1>, PD;
def NAME#32rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi32, 1>;
def NAME#64rr_ND_REV : BinOpRR_RF_Rev<BaseOpc2, mnemonic, Xi64, 1>;
def NAME#8rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi8>, NF;
def NAME#16rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi16>, NF, PD;
def NAME#32rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi32>, NF;
def NAME#64rr_NF_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi64>, NF;
def NAME#8rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi8, 1>, EVEX_NF;
def NAME#16rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi16, 1>, EVEX_NF, PD;
def NAME#32rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi32, 1>, EVEX_NF;
def NAME#64rr_NF_ND_REV : BinOpRR_R_Rev<BaseOpc2, mnemonic, Xi64, 1>, EVEX_NF;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems these are not tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the test for reverse encoding is missing. I will add.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added llvm/test/MC/Disassembler/X86/apx/reverse-encoding.txt

@KanRobert KanRobert changed the title [X86][MC] Support encoding/decoding for APX variant ADD/SUB/ADC/SBB/OR/XOR instructions [X86][MC] Support encoding/decoding for APX variant ADD/SUB/ADC/SBB/OR/XOR/NEG/NOT instructions Dec 27, 2023
Copy link
Contributor

@phoebewang phoebewang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@KanRobert
Copy link
Contributor Author

@phoebewang Thanks!

@KanRobert KanRobert merged commit d79ccee into llvm:main Dec 28, 2023
3 of 4 checks passed
mtrofin added a commit that referenced this pull request Dec 28, 2023
Some opcodes changed.
KanRobert added a commit that referenced this pull request Jan 11, 2024
…T/INC/DEC/IMUL (#77564)

We supported encoding/decoding for these instructions in

#76319
#76721
#76919
KanRobert added a commit that referenced this pull request Jan 16, 2024
We supported encoding/decoding for APX AND in #76319

This test should be added in #77564 but was missing.
qiaojbao pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Jan 26, 2024
…68192a2fc

Local branch amd-gfx 4566819 Merged main:eaa32d20a2612370371047140734e91f8f22dea1 into amd-gfx:7be0dd27ca83
Remote branch main a1f1371 [X86][NFC] Remove redundant constraints in X86InstrArithmetic.td after llvm#76319
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…T/INC/DEC/IMUL (llvm#77564)

We supported encoding/decoding for these instructions in

llvm#76319
llvm#76721
llvm#76919
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
We supported encoding/decoding for APX AND in llvm#76319

This test should be added in llvm#77564 but was missing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants