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

[LiuChen via Phabricator via llvm-commits]

LiuChen3 added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrFormats.td:356
   bit ExplicitVEXPrefix = 0; // Force the instruction to use VEX encoding.
+  bit checkPredicate = 0; // Does this VEX inst should check predicate?
 
----------------
pengfei wrote:
> checkVEXPredicate?
Maybe in the future we need check more Predicate than VEX?


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:7169
 //===----------------------------------------------------------------------===//
 let Predicates = [HasAVXVNNI, NoVLX_Or_NoVNNI], Constraints = "$src1 = $dst" in
 multiclass avx_vnni_rm<bits<8> opc, string OpcodeStr, SDNode OpNode,
----------------
pengfei wrote:
> Should be simple to add here i.e. `ExplicitVEXPrefix, checkPredicate = 1`
Of course we can do this.


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:7203
 
-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;
----------------
LuoYuanke wrote:
> Do we support customized code for CheckPredicate, so that we can extend the functionality someday? In this case the CheckPredicate is {ST->hasAVXVNNI();}.
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.


================
Comment at: llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp:23
+// List of the Predicates should be ignored.
+StringRef ExcludePredicates[] = {"OptForSize", "OptForSpeed", "NoAVX512",
+                                 "NoFMA4",     "NoBWI",       "NoVLX",
----------------
pengfei wrote:
> Can we assume the predicates that need to check all started with `HasAVXxxx`. Then we don't need such table.
Yes. It's good idea.


================
Comment at: llvm/utils/TableGen/X86EVEX2VEXTablesEmitter.cpp:44
+  // Represent predicates of VEX instructions.
+  std::vector<Predicate> EVEX2VEX128Predicates;
+  std::vector<Predicate> EVEX2VEX256Predicates;
----------------
pengfei wrote:
> Do we really need 2 tables?
You are right. We don't need 2 tables.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98011/new/

https://reviews.llvm.org/D98011