[X86][NFC] Adding one flag to imply whether the instruction should check the predicate when compress EVEX instructions to VEX encoding.

[LiuChen3]

Summary

Some EVEX instructions should check the predicates when compress to VEX
encoding. For example, avx512vnni instructions. This is because avx512vnni
doesn't mean that avxvnni is supported on the target.
This patch moving the manually added check to .inc that generated by tablegen.

[lebedev.ri]

Tests, and some words in description/patch, are missing.

[pengfei]

llvm/lib/Target/X86/X86InstrFormats.td

356

checkVEXPredicate?

llvm/lib/Target/X86/X86InstrSSE.td

7169

Should be simple to add here i.e. ExplicitVEXPrefix, checkPredicate = 1

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

23

Can we assume the predicates that need to check all started with HasAVXxxx. Then we don't need such table.

44

Do we really need 2 tables?

202

If we can assume the predicates that need to check all started with HasAVXxxx, then we just need to find it in the PredicatesRecords and check it only.

[LuoYuanke]

llvm/lib/Target/X86/X86InstrSSE.td

7203

Do we support customized code for CheckPredicate, so that we can extend the functionality someday? In this case the CheckPredicate is {ST->hasAVXVNNI();}.

[craig.topper]

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

204

releated -> related

[LiuChen3]

llvm/lib/Target/X86/X86InstrFormats.td

356

Maybe in the future we need check more Predicate than VEX?

llvm/lib/Target/X86/X86InstrSSE.td

7169

Of course we can do this.

7203

I think if you want to add customized check, you can add Predicate like "def HasAVXVNNI". Than this predicate can include in the table EVEX2VEX[128/256]Predicates.

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

23

Yes. It's good idea.

44

You are right. We don't need 2 tables.

[LiuChen3]

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

202

Related to Yuanke's question, if support customized code for CheckPredicate, the name could be something else, so that we can't depend on the whether the predicate starts with "HasAVX". This is also my first thought. What's your opinion here? Of course we can apply your method firstly and added more conditions in the future.

[pengfei]

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

202

I'm not sure what's customized for in future. I can see its only required during this EVEX to VEX transformation. Any thoughts @LuoYuanke ?

[LiuChen3]

In D98011#2605726, @lebedev.ri wrote:

Tests, and some words in description/patch, are missing.

Thanks for your review, @lebedev.ri. This is actually one NFC patch. I think we don't need add new tests.

[LuoYuanke]

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

202

I'm just thinking if the predict can be extensible. I agree we can apply current approach first and refactor it if any extension is needed.

[LiuChen3]

Rebase and address Pengfei and Craig's comments.

[LiuChen3]

llvm/lib/Target/X86/X86InstrSSE.td

7169

I am sorry that I forgot this!

[LiuChen3]

Address pengfei's comment.

[pengfei]

llvm/lib/Target/X86/X86EvexToVex.cpp

255–259

Don't need to reformat the unrelated code.

llvm/lib/Target/X86/X86InstrFormats.td

239

This is not needed.

356

I think you can add VEX for now since you mentioned on the comment too.

llvm/lib/Target/X86/X86InstrSSE.td

7203–7206

These are not needed.

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

206–207

Do you have cases that have more than one HasAVXxxx in Predicates?

209

Not necessary?

213–214

We don't need to add case XXX return ture since we use the default: return true;.
And since we always need to check predicate when we add it in the td file, we don't expect the Predicates is empty. I think we can add a assert for this case.

[LiuChen3]

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

213–214

Yes. The default value should be 'llvm_unreachable'. In this case, "return true" is needed.

[LiuChen3]

Address comments

[pengfei]

LGTM.

llvm/lib/Target/X86/X86InstrFormats.td

356

Nit: Not a good sentences. How about:
Force to check predicate before using VEX encoding.

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

58

Nit: Add comments like above.

202

Nit: Maybe bettter to move the comments above the for or add curly brackets for for.

[lebedev.ri]

In D98011#2610381, @LiuChen3 wrote:

In D98011#2605726, @lebedev.ri wrote:

Tests, and some words in description/patch, are missing.

Thanks for your review, @lebedev.ri. This is actually one NFC patch. I think we don't need add new tests.

How is it NFC?

[pengfei]

In D98011#2610900, @lebedev.ri wrote:

In D98011#2610381, @LiuChen3 wrote:

In D98011#2605726, @lebedev.ri wrote:

Tests, and some words in description/patch, are missing.

Thanks for your review, @lebedev.ri. This is actually one NFC patch. I think we don't need add new tests.

How is it NFC?

I think @LiuChen3 's NFC firstly means to LLVM. I.e. Moving the manually added check to .inc that generated by tablegen. It's hard to say NFC for the tablegen code. But with a coarse search, I didn't find we have a test for this EVEX2VEX backend. So I assumed it is covered by LLVM tests. By this mean, I think we can call it a NFC :)

[lebedev.ri]

In D98011#2610939, @pengfei wrote:

In D98011#2610900, @lebedev.ri wrote:

In D98011#2610381, @LiuChen3 wrote:

In D98011#2605726, @lebedev.ri wrote:

Tests, and some words in description/patch, are missing.

Thanks for your review, @lebedev.ri. This is actually one NFC patch. I think we don't need add new tests.

How is it NFC?

I think @LiuChen3 's NFC firstly means to LLVM. I.e. Moving the manually added check to .inc that generated by tablegen. It's hard to say NFC for the tablegen code. But with a coarse search, I didn't find we have a test for this EVEX2VEX backend. So I assumed it is covered by LLVM tests. By this mean, I think we can call it a NFC :)

That should be explained in patch's description :)

[pengfei]

In D98011#2610940, @lebedev.ri wrote:

In D98011#2610939, @pengfei wrote:

In D98011#2610900, @lebedev.ri wrote:

In D98011#2610381, @LiuChen3 wrote:

In D98011#2605726, @lebedev.ri wrote:

Tests, and some words in description/patch, are missing.

Thanks for your review, @lebedev.ri. This is actually one NFC patch. I think we don't need add new tests.

How is it NFC?

I think @LiuChen3 's NFC firstly means to LLVM. I.e. Moving the manually added check to .inc that generated by tablegen. It's hard to say NFC for the tablegen code. But with a coarse search, I didn't find we have a test for this EVEX2VEX backend. So I assumed it is covered by LLVM tests. By this mean, I think we can call it a NFC :)

That should be explained in patch's description :)

Agreed. Thanks.

[LiuChen3]

In D98011#2610944, @pengfei wrote:

In D98011#2610940, @lebedev.ri wrote:

In D98011#2610939, @pengfei wrote:

In D98011#2610900, @lebedev.ri wrote:

In D98011#2610381, @LiuChen3 wrote:

In D98011#2605726, @lebedev.ri wrote:

Tests, and some words in description/patch, are missing.

Thanks for your review, @lebedev.ri. This is actually one NFC patch. I think we don't need add new tests.

How is it NFC?

I think @LiuChen3 's NFC firstly means to LLVM. I.e. Moving the manually added check to .inc that generated by tablegen. It's hard to say NFC for the tablegen code. But with a coarse search, I didn't find we have a test for this EVEX2VEX backend. So I assumed it is covered by LLVM tests. By this mean, I think we can call it a NFC :)

That should be explained in patch's description :)

Agreed. Thanks.

It's my fault. Thanks for all of you.

[LiuChen3]

Address pengfei's comments

llvm/lib/Target/X86/X86EvexToVex.cpp

 }
 
 // Do any custom cleanup needed to finalize the conversion.
 static bool performCustomAdjustments(MachineInstr &MI, unsigned NewOpc,
                                      const X86Subtarget *ST) {
   (void)NewOpc;
   unsigned Opc = MI.getOpcode();
   switch (Opc) {
-  case X86::VPDPBUSDSZ256m:
-  case X86::VPDPBUSDSZ256r:
-  case X86::VPDPBUSDSZ128m:
-  case X86::VPDPBUSDSZ128r:
-  case X86::VPDPBUSDZ256m:
-  case X86::VPDPBUSDZ256r:
-  case X86::VPDPBUSDZ128m:
-  case X86::VPDPBUSDZ128r:
-  case X86::VPDPWSSDSZ256m:
-  case X86::VPDPWSSDSZ256r:
-  case X86::VPDPWSSDSZ128m:
-  case X86::VPDPWSSDSZ128r:
-  case X86::VPDPWSSDZ256m:
-  case X86::VPDPWSSDZ256r:
-  case X86::VPDPWSSDZ128m:
-  case X86::VPDPWSSDZ128r:
-    // These can only VEX convert if AVXVNNI is enabled.
-    return ST->hasAVXVNNI();
   case X86::VALIGNDZ128rri:
   case X86::VALIGNDZ128rmi:
   case X86::VALIGNQZ128rri:
   case X86::VALIGNQZ128rmi: {
     assert((NewOpc == X86::VPALIGNRrri || NewOpc == X86::VPALIGNRrmi) &&
            "Unexpected new opcode!");
     unsigned Scale = (Opc == X86::VALIGNQZ128rri ||
                       Opc == X86::VALIGNQZ128rmi) ? 8 : 4;
if (!TableChecked.load(std::memory_order_relaxed)) {
     assert(llvm::is_sorted(X86EvexToVex128CompressTable) &&
            "X86EvexToVex128CompressTable is not sorted!");
     assert(llvm::is_sorted(X86EvexToVex256CompressTable) &&
            "X86EvexToVex256CompressTable is not sorted!");
     TableChecked.store(true, std::memory_order_relaxed);
   }
 #endif
 
-  // Use the VEX.L bit to select the 128 or 256-bit table.
+  // Use the VEX.L bit to select the 128 or 256-bit table and predicate-check
+  // function.
+  bool IsVEXLInst = Desc.TSFlags & X86II::VEX_L;
   ArrayRef<X86EvexToVexCompressTableEntry> Table =
-    (Desc.TSFlags & X86II::VEX_L) ? makeArrayRef(X86EvexToVex256CompressTable)
-                                  : makeArrayRef(X86EvexToVex128CompressTable);
+      IsVEXLInst ? makeArrayRef(X86EvexToVex256CompressTable)
+  bool (*CheckVEXInstPredicate)(MachineInstr & MI, const X86Subtarget *) =
+      IsVEXLInst ? CheckVEXInstPredicate256 : CheckVEXInstPredicate128;
 
   const auto *I = llvm::lower_bound(Table, MI.getOpcode());
   if (I == Table.end() || I->EvexOpcode != MI.getOpcode())
     return false;
 
   unsigned NewOpc = I->VexOpcode;
 
   if (usesExtendedRegister(MI))
     return false;
 
+  if (!CheckVEXInstPredicate(MI, ST))
+    return false;
+
   if (!performCustomAdjustments(MI, NewOpc, ST))
     return false;
 
   MI.setDesc(TII->get(NewOpc));
   MI.setAsmPrinterFlag(X86::AC_EVEX_2_VEX);
   return true;
 }
 
 INITIALIZE_PASS(EvexToVexInstPass, EVEX2VEX_NAME, EVEX2VEX_DESC, false, false)
 
 FunctionPass *llvm::createX86EvexToVexInsts() {
   return new EvexToVexInstPass();
 }

llvm/lib/Target/X86/X86InstrFormats.td

 class EVEX_KZ : EVEX_K { bit hasEVEX_Z = 1; }
 class EVEX_B { bit hasEVEX_B = 1; }
 class EVEX_RC { bit hasEVEX_RC = 1; }
 class EVEX_V512 { bit hasEVEX_L2 = 1; bit hasVEX_L = 0; }
 class EVEX_V256 { bit hasEVEX_L2 = 0; bit hasVEX_L = 1; }
 class EVEX_V128 { bit hasEVEX_L2 = 0; bit hasVEX_L = 0; }
 class NOTRACK { bit hasNoTrackPrefix = 1; }
 class SIMD_EXC { list<Register> Uses = [MXCSR]; bit mayRaiseFPException = 1; }
+class CheckPredicate { bit checkPredicate = 1; }
 
 // Specify AVX512 8-bit compressed displacement encoding based on the vector
 // element size in bits (8, 16, 32, 64) and the CDisp8 form.
 class EVEX_CD8<int esize, CD8VForm form> {
   int CD8_EltSize = !srl(esize, 3);
   bits<3> CD8_Form = form.Value;
 }
 
class X86Inst<bits<8> opcod, Format f, ImmType i, dag outs, dag ins,
   string FoldGenRegForm = ?;
 
   // Used to prevent an explicit EVEX2VEX override for this instruction.
   string EVEX2VEXOverride = ?;
 
   bit isMemoryFoldable = 1;     // Is it allowed to memory fold/unfold this instruction?
   bit notEVEX2VEXConvertible = 0; // Prevent EVEX->VEX conversion.
   bit ExplicitVEXPrefix = 0; // Force the instruction to use VEX encoding.
+  bit checkPredicate = 0; // Does this VEX inst should check predicate?
 
   // TSFlags layout should be kept in sync with X86BaseInfo.h.
   let TSFlags{6-0}   = FormBits;
   let TSFlags{8-7}   = OpSizeBits;
   let TSFlags{10-9}  = AdSizeBits;
   // No need for 3rd bit, we don't need to distinguish NoPrfx from PS.
   let TSFlags{12-11} = OpPrefixBits{1-0};
   let TSFlags{15-13} = OpMapBits;

llvm/lib/Target/X86/X86InstrSSE.td

defm VMASKMOVPD : avx_movmask_rm<0x2D, 0x2F, "vmaskmovpd",
                                  int_x86_avx_maskload_pd_256,
                                  int_x86_avx_maskstore_pd,
                                  int_x86_avx_maskstore_pd_256,
                                  WriteFMaskMove64, WriteFMaskMove64Y>;
 
 //===----------------------------------------------------------------------===//
 // AVX_VNNI
 //===----------------------------------------------------------------------===//
 let Predicates = [HasAVXVNNI, NoVLX_Or_NoVNNI], Constraints = "$src1 = $dst" in
 multiclass avx_vnni_rm<bits<8> opc, string OpcodeStr, SDNode OpNode,
                        bit IsCommutable> {
   let isCommutable = IsCommutable in
   def rr  : AVX8I<opc, MRMSrcReg, (outs VR128:$dst),
              (ins VR128:$src1, VR128:$src2, VR128:$src3),
              !strconcat(OpcodeStr, "\t{$src3, $src2, $dst|$dst, $src2, $src3}"),
              [(set VR128:$dst, (v4i32 (OpNode VR128:$src1,
                                        VR128:$src2, VR128:$src3)))]>,
multiclass avx_vnni_rm<bits<8> opc, string OpcodeStr, SDNode OpNode,
   def Yrm  : AVX8I<opc, MRMSrcMem, (outs VR256:$dst),
              (ins VR256:$src1, VR256:$src2, i256mem:$src3),
              !strconcat(OpcodeStr, "\t{$src3, $src2, $dst|$dst, $src2, $src3}"),
              [(set VR256:$dst, (v8i32 (OpNode VR256:$src1, VR256:$src2,
                                       (loadv8i32 addr:$src3))))]>,
              VEX_4V, VEX_L, Sched<[SchedWriteVecIMul.XMM]>;
 }
 
-defm VPDPBUSD   : avx_vnni_rm<0x50, "vpdpbusd", X86Vpdpbusd, 0>, ExplicitVEXPrefix;
-defm VPDPBUSDS  : avx_vnni_rm<0x51, "vpdpbusds", X86Vpdpbusds, 0>, ExplicitVEXPrefix;
-defm VPDPWSSD   : avx_vnni_rm<0x52, "vpdpwssd",  X86Vpdpwssd, 1>, ExplicitVEXPrefix;
-defm VPDPWSSDS  : avx_vnni_rm<0x53, "vpdpwssds", X86Vpdpwssds, 1>, ExplicitVEXPrefix;
+defm VPDPBUSD   : avx_vnni_rm<0x50, "vpdpbusd", X86Vpdpbusd, 0>, ExplicitVEXPrefix, CheckPredicate;
+defm VPDPBUSDS  : avx_vnni_rm<0x51, "vpdpbusds", X86Vpdpbusds, 0>, ExplicitVEXPrefix, CheckPredicate;
+defm VPDPWSSD   : avx_vnni_rm<0x52, "vpdpwssd",  X86Vpdpwssd, 1>, ExplicitVEXPrefix, CheckPredicate;
+defm VPDPWSSDS  : avx_vnni_rm<0x53, "vpdpwssds", X86Vpdpwssds, 1>, ExplicitVEXPrefix, CheckPredicate;
 
 def X86vpmaddwd_su : PatFrag<(ops node:$lhs, node:$rhs),
                              (X86vpmaddwd node:$lhs, node:$rhs), [{
   return N->hasOneUse();
 }]>;
 
 let Predicates = [HasAVXVNNI, NoVLX_Or_NoVNNI] in {
   def : Pat<(v8i32 (add VR256:$src1,

llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp

 #include "CodeGenTarget.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/TableGenBackend.h"
 
 using namespace llvm;
 
 namespace {
 
+// List of the Predicates should be ignored.
+StringRef ExcludePredicates[] = {"OptForSize", "OptForSpeed", "NoAVX512",
+                                 "NoFMA4",     "NoBWI",       "NoVLX",
+                                 "NoDQI"};
+
 class X86EVEX2VEXTablesEmitter {
   RecordKeeper &Records;
   CodeGenTarget Target;
 
   // Hold all non-masked & non-broadcasted EVEX encoded instructions
   std::vector<const CodeGenInstruction *> EVEXInsts;
   // Hold all VEX encoded instructions. Divided into groups with same opcodes
   // to make the search more efficient
   std::map<uint64_t, std::vector<const CodeGenInstruction *>> VEXInsts;
 
   typedef std::pair<const CodeGenInstruction *, const CodeGenInstruction *> Entry;
+  typedef std::pair<StringRef, const std::string> Predicate;
 
   // Represent both compress tables
   std::vector<Entry> EVEX2VEX128;
   std::vector<Entry> EVEX2VEX256;
+  // Represent predicates of VEX instructions.
+  std::vector<Predicate> EVEX2VEX128Predicates;
+  std::vector<Predicate> EVEX2VEX256Predicates;
 
 public:
   X86EVEX2VEXTablesEmitter(RecordKeeper &R) : Records(R), Target(R) {}
 
   // run - Output X86 EVEX2VEX tables.
   void run(raw_ostream &OS);
 
 private:
   // Prints the given table as a C++ array of type
   // X86EvexToVexCompressTableEntry
   void printTable(const std::vector<Entry> &Table, raw_ostream &OS);
+
+  void printCheckPredicate(const std::vector<Predicate> &Predicates,
+                           raw_ostream &OS);
 };
 
 void X86EVEX2VEXTablesEmitter::printTable(const std::vector<Entry> &Table,
                                           raw_ostream &OS) {
   StringRef Size = (Table == EVEX2VEX128) ? "128" : "256";
 
   OS << "// X86 EVEX encoded instructions that have a VEX " << Size
      << " encoding\n"
      << "// (table format: <EVEX opcode, VEX-" << Size << " opcode>).\n"
      << "static const X86EvexToVexCompressTableEntry X86EvexToVex" << Size
      << "CompressTable[] = {\n"
      << "  // EVEX scalar with corresponding VEX.\n";
 
   // Print all entries added to the table
   for (auto Pair : Table) {
     OS << "  { X86::" << Pair.first->TheDef->getName()
        << ", X86::" << Pair.second->TheDef->getName() << " },\n";
   }
 
   OS << "};\n\n";
 }
 
+void X86EVEX2VEXTablesEmitter::printCheckPredicate(
+    const std::vector<Predicate> &Predicates, raw_ostream &OS) {
+  StringRef Size = (Predicates == EVEX2VEX128Predicates) ? "128" : "256";
+  OS << "static bool CheckVEXInstPredicate" << Size
+     << "(MachineInstr &MI, const X86Subtarget *Subtarget) {\n"
+     << "  unsigned Opc = MI.getOpcode();\n"
+     << "  switch (Opc) {\n"
+     << "    default: return true;\n";
+  for (auto Pair : Predicates)
+    OS << "    case X86::" << Pair.first << ": return " << Pair.second << ";\n";
+  OS << "  }\n"
+     << "}\n\n";
+}
+
 // Return true if the 2 BitsInits are equal
 // Calculates the integer value residing BitsInit object
 static inline uint64_t getValueFromBitsInit(const BitsInit *B) {
   uint64_t Value = 0;
   for (unsigned i = 0, e = B->getNumBits(); i != e; ++i) {
     if (BitInit *Bit = dyn_cast<BitInit>(B->getBit(i)))
       Value |= uint64_t(Bit->getValue()) << i;
     else
static inline unsigned int getRegOperandSize(const Record *RegRec) {
     if (RegRec->isSubClassOf("RegisterOperand"))
       return RegRec->getValueAsDef("RegClass")->getValueAsInt("Alignment");
 
     llvm_unreachable("Register operand's size not known!");
   }
 };
 
 void X86EVEX2VEXTablesEmitter::run(raw_ostream &OS) {
+  auto getPredicates = [&](const CodeGenInstruction *Inst) {
+    std::string Predicates;
+    std::vector<Record *> PredicatesRecords =
+        Inst->TheDef->getValueAsListOfDefs("Predicates");
+    for (unsigned i = 0, e = PredicatesRecords.size(); i != e; ++i) {
+      StringRef PredicatesRecordsName = PredicatesRecords[i]->getName();
+      // Skip the predicates of SelectionDAG and AVX512 releated feature check.
+      if (any_of(ExcludePredicates, [&](StringRef ExcludePredicate) {
+            return PredicatesRecordsName.find(ExcludePredicate) !=
+                   StringRef::npos;
+          }))
+        continue;
+      if (!Predicates.empty())
+        Predicates += " && ";
+      Predicates += '(' +
+                    PredicatesRecords[i]->getValueAsString("CondString").str() +
+                    ')';
+    }
+    if (Predicates.empty())
+      Predicates = "true";
+    return Predicates;
+  };
+
   emitSourceFileHeader("X86 EVEX2VEX tables", OS);
 
   ArrayRef<const CodeGenInstruction *> NumberedInstructions =
       Target.getInstructionsByEnumValue();
 
   for (const CodeGenInstruction *Inst : NumberedInstructions) {
     // Filter non-X86 instructions.
     if (!Inst->TheDef->isSubClassOf("X86Inst"))
if (!EVEXInst->TheDef->isValueUnset("EVEX2VEXOverride")) {
       if (Match != VEXInsts[Opcode].end())
         VEXInst = *Match;
     }
 
     if (!VEXInst)
       continue;
 
     // In case a match is found add new entry to the appropriate table
-    if (EVEXInst->TheDef->getValueAsBit("hasVEX_L"))
+    if (EVEXInst->TheDef->getValueAsBit("hasVEX_L")) {
       EVEX2VEX256.push_back(std::make_pair(EVEXInst, VEXInst)); // {0,1}
-    else
+      if (VEXInst->TheDef->getValueAsBit("checkPredicate"))
+        EVEX2VEX256Predicates.push_back(std::make_pair(
+            EVEXInst->TheDef->getName(), getPredicates(VEXInst)));
+    } else {
       EVEX2VEX128.push_back(std::make_pair(EVEXInst, VEXInst)); // {0,0}
+      if (VEXInst->TheDef->getValueAsBit("checkPredicate"))
+        EVEX2VEX128Predicates.push_back(std::make_pair(
+            EVEXInst->TheDef->getName(), getPredicates(VEXInst)));
+    }
   }
 
   // Print both tables
   printTable(EVEX2VEX128, OS);
   printTable(EVEX2VEX256, OS);
+  printCheckPredicate(EVEX2VEX128Predicates, OS);
+  printCheckPredicate(EVEX2VEX256Predicates, OS);
 }
 }
 
 namespace llvm {
 void EmitX86EVEX2VEXTables(RecordKeeper &RK, raw_ostream &OS) {
   X86EVEX2VEXTablesEmitter(RK).run(OS);
 }
 }