[PATCH] D135933: [X86] Add CMPCCXADD instructions.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Basic/Targets/X86.cpp:781
+    Builder.defineMacro("__CMPCCXADD__");
+  Builder.defineMacro("__CMPCCXADD_SUPPORTED__");
   if (HasAVXVNNI)
----------------
What is __CMPCCXADD_SUPPORTED__ for?


================
Comment at: llvm/lib/Target/X86/X86.td:259
+                                        "Support CMPCCXADD instructions",
+                                        [FeatureAVX2]>;
 def FeatureINVPCID : SubtargetFeature<"invpcid", "HasINVPCID", "true",
----------------
Why AVX2?


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8118
 
+let Predicates = [HasCMPCCXADD, In64BitMode], Constraints = "$dstsrc2 = $dst" in
+multiclass CMPCCXADD_BASE<bits<8> Opc, string OpcodeStr> {
----------------
This feels like it belongs somewhere other than X86InstrSSE.td since it's not vector related.


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8131
+
+defm CMPBEXADD : CMPCCXADD_BASE<0xe6, "cmpbexadd">;
+defm CMPBXADD  : CMPCCXADD_BASE<0xe2, "cmpbxadd">;
----------------
Any possibility of doing this like how JCC_1, SETCCr, and CMOV32rr using an immediate for the lower 4 bits of the opcode?


================
Comment at: llvm/lib/Target/X86/X86InstrSSE.td:8145-8146
+defm CMPPXADD : CMPCCXADD_BASE<0xea, "cmppxadd">;
+defm CMPSXADD : CMPCCXADD_BASE<0xe8, "cmpsxadd">;
+defm CMPZXADD : CMPCCXADD_BASE<0xe4, "cmpzxadd">;
+
----------------
Should there be aliases for consistency with Jcc, Setcc, and cmovcc. To support A, AE, GT, GE etc.?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135933