[X86] Prefer VEX encoding in X86 assembler.

[craig.topper]

Did you consider other patches like this which will sort everything X86InstrAVX512.td after X86InstrSSE.td? The current sorting is done by the instruction name I think so VCVTSD2SIZrm_Int sorts before VCVTSD2SIrm_Int.

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 00bdd127e3c2..aaf90aa13272 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -636,7 +636,7 @@ struct MatchableInfo {
     if (RequiredFeatures.size() != RHS.RequiredFeatures.size())
       return RequiredFeatures.size() > RHS.RequiredFeatures.size();
 
-    return false;
+    return TheDef->getID() < RHS.TheDef->getID();
   }
 
   /// couldMatchAmbiguouslyWith - Check whether this matchable could

[LuoYuanke]

In D111505#3054704, @craig.topper wrote:

Did you consider other patches like this which will sort everything X86InstrAVX512.td after X86InstrSSE.td? The current sorting is done by the instruction name I think so VCVTSD2SIZrm_Int sorts before VCVTSD2SIrm_Int.

diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index 00bdd127e3c2..aaf90aa13272 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -636,7 +636,7 @@ struct MatchableInfo {
     if (RequiredFeatures.size() != RHS.RequiredFeatures.size())
       return RequiredFeatures.size() > RHS.RequiredFeatures.size();
 
-    return false;
+    return TheDef->getID() < RHS.TheDef->getID();
   }
 
   /// couldMatchAmbiguouslyWith - Check whether this matchable could

Great. I once seek to reorder the matching table, but I didn't realize "TheDef->getID()" can achieve it.

[LuoYuanke]

Thanks, Craig. I created another patch at https://reviews.llvm.org/D111538.

[LuoYuanke]

Change back the order of checkAsmTiedOperandConstraints() and checkTargetMatchPredicate().

[skan]

The Match_NotPerfect mechanism slows down the match process. I think D111576 is better.

[LuoYuanke]

In D111505#3057532, @skan wrote:

The Match_NotPerfect mechanism slows down the match process. I think D111576 is better.

I think https://reviews.llvm.org/D111538 is the best.

llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h

 //===- llvm/MC/MCTargetAsmParser.h - Target Assembly Parser -----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 

 public:
   enum MatchResultTy {
     Match_InvalidOperand,
     Match_InvalidTiedOperand,
     Match_MissingFeature,
     Match_MnemonicFail,
     Match_Success,
     Match_NearMisses,
+    Match_NotPerfect,
     FIRST_TARGET_MATCH_RESULT_TY
   };
 
 protected: // Can only create subclasses.
   MCTargetAsmParser(MCTargetOptions const &, const MCSubtargetInfo &STI,
                     const MCInstrInfo &MII);
 
   /// Create a copy of STI and return a non-const reference to it.

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

 //===-- X86AsmParser.cpp - Parse X86 assembly to MCInst instructions ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
unsigned X86AsmParser::checkTargetMatchPredicate(MCInst &Inst) {
 
   // These instructions are only available with {vex}, {vex2} or {vex3} prefix
   if (MCID.TSFlags & X86II::ExplicitVEXPrefix &&
       (ForcedVEXEncoding != VEXEncoding_VEX &&
        ForcedVEXEncoding != VEXEncoding_VEX2 &&
        ForcedVEXEncoding != VEXEncoding_VEX3))
     return Match_Unsupported;
 
-  // These instructions match ambiguously with their VEX encoded counterparts
-  // and appear first in the matching table. Reject them unless we're forcing
-  // EVEX encoding.
-  // FIXME: We really need a way to break the ambiguity.
-  switch (Opc) {
-  case X86::VCVTSD2SIZrm_Int:
-  case X86::VCVTSD2SI64Zrm_Int:
-  case X86::VCVTSS2SIZrm_Int:
-  case X86::VCVTSS2SI64Zrm_Int:
-  case X86::VCVTTSD2SIZrm:   case X86::VCVTTSD2SIZrm_Int:
-  case X86::VCVTTSD2SI64Zrm: case X86::VCVTTSD2SI64Zrm_Int:
-  case X86::VCVTTSS2SIZrm:   case X86::VCVTTSS2SIZrm_Int:
-  case X86::VCVTTSS2SI64Zrm: case X86::VCVTTSS2SI64Zrm_Int:
-    if (ForcedVEXEncoding != VEXEncoding_EVEX)
-      return Match_Unsupported;
-    break;
-  }
+  // We prefer vex encoding instead of evex by default. For some instructions
+  // evex instruction is ahead of vex in the matching table. We give a chance
+  // to match vex instruction for those cases (e.g. VCVTSD2SIZrm_Int,
+  // VCVTTSD2SIZrm, VCVTTSS2SI64Zrm).
+  if (ForcedVEXEncoding == VEXEncoding_Default &&
+      (MCID.TSFlags & X86II::EncodingMask) == X86II::EVEX)
+    return Match_NotPerfect;
 
   return Match_Success;
 }
 
 bool X86AsmParser::MatchAndEmitATTInstruction(SMLoc IDLoc, unsigned &Opcode,
                                               OperandVector &Operands,
                                               MCStreamer &Out,
                                               uint64_t &ErrorInfo,

llvm/utils/TableGen/AsmMatcherEmitter.cpp

 //===- AsmMatcherEmitter.cpp - Generate an assembly matcher ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
if (!ReportMultipleNearMisses) {
     OS << "  if (Operands.size() > "
        << (MaxNumOperands + HasMnemonicFirst) << ") {\n";
     OS << "    ErrorInfo = "
        << (MaxNumOperands + HasMnemonicFirst) << ";\n";
     OS << "    return Match_InvalidOperand;\n";
     OS << "  }\n\n";
   }
 
+  OS << "  SmallVector<MCInst, 2> Candidates;\n";
   // Emit code to get the available features.
   OS << "  // Get the current feature set.\n";
   OS << "  const FeatureBitset &AvailableFeatures = getAvailableFeatures();\n\n";
 
   OS << "  // Get the instruction mnemonic, which is the first token.\n";
   if (HasMnemonicFirst) {
     OS << "  StringRef Mnemonic = ((" << Target.getName()
        << "Operand &)*Operands[0]).getToken();\n\n";
#endif
   OS << "\n";
 
   // Verify the instruction with the target-specific match predicate function.
   OS << "    // We have a potential match. Check the target predicate to\n"
      << "    // handle any context sensitive constraints.\n"
      << "    if ((MatchResult = checkTargetMatchPredicate(Inst)) !="
      << " Match_Success) {\n"
      << "      DEBUG_WITH_TYPE(\"asm-matcher\",\n"
-     << "                      dbgs() << \"Target match predicate failed with diag code \"\n"
+     << "                      dbgs() << \"Target match predicate failed with "
+        "diag code \"\n"
      << "                             << MatchResult << \"\\n\");\n"
+     << "      if (MatchResult == Match_NotPerfect)\n"
+     << "         Candidates.push_back(Inst);\n"
      << "      Inst.clear();\n";
   if (ReportMultipleNearMisses) {
-    OS << "      LatePredicateNearMiss = NearMissInfo::getMissedPredicate(MatchResult);\n";
+    OS << "      LatePredicateNearMiss = "
+          "NearMissInfo::getMissedPredicate(MatchResult);\n";
   } else {
     OS << "      RetCode = MatchResult;\n"
        << "      HadMatchOtherThanPredicate = true;\n"
        << "      continue;\n";
   }
   OS << "    }\n\n";
 
   if (ReportMultipleNearMisses) {
#endif
   }
 
   OS << "    DEBUG_WITH_TYPE(\n";
   OS << "        \"asm-matcher\",\n";
   OS << "        dbgs() << \"Opcode result: complete match, selecting this opcode\\n\");\n";
   OS << "    return Match_Success;\n";
   OS << "  }\n\n";
 
+  OS << "  if (!Candidates.empty()) {\n";
+  OS << "    // TODO Just pick up the lasted candidate, we can fine grain it "
+        "later\n";
+  OS << "    auto Candidate = Candidates.back();\n";
+  OS << "    Inst = Candidate;\n";
+  OS << "    return Match_Success;\n";
+  OS << "  }\n";
+
   if (ReportMultipleNearMisses) {
     OS << "  // No instruction variants matched exactly.\n";
     OS << "  return Match_NearMisses;\n";
   } else {
     OS << "  // Okay, we had no match.  Try to return a useful error code.\n";
     OS << "  if (HadMatchOtherThanPredicate || !HadMatchOtherThanFeatures)\n";
     OS << "    return RetCode;\n\n";
     OS << "  ErrorInfo = 0;\n";