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

[Freddy, Ye via Phabricator via llvm-commits]

FreddyYe added inline comments.


================
Comment at: clang/lib/Headers/cmpccxaddintrin.h:19-34
+  _CMPCCX_O,   /* Overflow.  */
+  _CMPCCX_NO,  /* No overflow.  */
+  _CMPCCX_B,   /* Below.  */
+  _CMPCCX_NB,  /* Not below.  */
+  _CMPCCX_Z,   /* Zero.  */
+  _CMPCCX_NZ,  /* Not zero.  */
+  _CMPCCX_BE,  /* Below or equal.  */
----------------
craig.topper wrote:
> skan wrote:
> > Could you use the same suffix for the condition code as `./llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h`? e.g
> > ```
> > NB->AE
> > Z->E
> > NZ->NE
> > NBE->A
> > ```
> > and so on.
> Probably should have both versions as aliases.
Yes, agree to add both,


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.td:3035
+          "cmp${cond}xadd\t{$src3, $dst, $dstsrc1|$dstsrc1, $dst, $src3}",
+          [(set GR64:$dst, (X86cmpccxadd addr:$dstsrc1, GR64:$dstsrc2, GR64:$src3, timm:$cond))]>,
+          VEX_4V, VEX_W, T8PD, Sched<[WriteXCHG]>;
----------------
craig.topper wrote:
> skan wrote:
> > `set GR64:$dst, EFLAGS ...`?
> That doesn't work unless X86cmpccxadd is declare as having two results.
I did a test. It won't affect this intrinsic's lowering. Since it's not be useful and only for intrinsic lowering, I preferred to not add.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135933