[PATCH] D135933: [X86] Add CMPCCXADD instructions.
Freddy, Ye via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 23 22:41:31 PDT 2022
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
More information about the cfe-commits
mailing list