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

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added inline comments.


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


================
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> {
----------------
craig.topper wrote:
> craig.topper wrote:
> > This feels like it belongs somewhere other than X86InstrSSE.td since it's not vector related.
> Missing `Defs = [EFLAGS]` I think
Yes. Moved to llvm/lib/Target/X86/X86InstrCompiler.td


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


================
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">;
+
----------------
craig.topper wrote:
> Should there be aliases for consistency with Jcc, Setcc, and cmovcc. To support A, AE, GT, GE etc.?
Yes, changed so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135933