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

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: clang/test/Driver/x86-target-features.c:313
 
+// RUN: %clang -target i686-unknown-linux-gnu -mcmpccxadd %s -### -o %t.o 2>&1 | FileCheck -check-prefix=CMPCCXADD %s
+// RUN: %clang -target i686-unknown-linux-gnu -mno-cmpccxadd %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-CMPCCXADD %s
----------------
Use -target=xxx -march=xxx to match with others. xxx can be x86_64 I think.


================
Comment at: clang/test/Driver/x86-target-features.c:314
+// RUN: %clang -target i686-unknown-linux-gnu -mcmpccxadd %s -### -o %t.o 2>&1 | FileCheck -check-prefix=CMPCCXADD %s
+// RUN: %clang -target i686-unknown-linux-gnu -mno-cmpccxadd %s -### -o %t.o 2>&1 | FileCheck -check-prefix=NO-CMPCCXADD %s
+// CMPCCXADD: "-target-feature" "+cmpccxadd"
----------------
ditto.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:1073
       // operand.
-      if (NumOps == 8 &&
+      if ((NumOps == 8) &&
           Desc.getOperandConstraint(6, MCOI::TIED_TO) == 0)
----------------
Unrelated change.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:1127
     case X86II::MRMSrcMem4VOp3:
+    case X86II::MRMDestMem4VOp3CC:
       // Skip registers encoded in reg.
----------------
Maybe move it to line 1133? `MRMDestMem4VOp3CC` is used for VEX encoding only.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:816
+    VEX_R = ~(RegEnc >> 3) & 1;
+    EVEX_R2 = ~(RegEnc >> 4) & 1;
+
----------------
Also this is not needed.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:820
+    VEX_4V = ~VRegEnc & 0xf;
+    EVEX_V2 = ~(VRegEnc >> 4) & 1;
+    break;
----------------
ditto.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:34174
+  NODE_NAME_CASE(CMPCCXADD32)
+  NODE_NAME_CASE(CMPCCXADD64)
   NODE_NAME_CASE(TESTUI)
----------------
ditto.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.h:872
 
+    CMPCCXADD32,
+    CMPCCXADD64,
----------------
Add comments for them?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.h:873
+    CMPCCXADD32,
+    CMPCCXADD64,
+
----------------
This node seems not used.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.td:348
 
+def X86cmpccxadd : SDNode<"X86ISD::CMPCCXADD32", SDTX86Cmpccxadd,
+                          [SDNPHasChain, SDNPMayLoad, SDNPMayStore,
----------------
Change to `CMPCCXADD` since it's shared.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135933