[PATCH] D68632: [X86] Make memcmp() use PTEST if possible and also enable AVX1

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:20964
 
+  // Use PTEST when explicitly requested.
+  if (Op0.getOpcode() == X86ISD::PTEST && isNullConstant(Op1) &&
----------------
Remove this. It doesn't make sense.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:42403
+      if (VecVT == CmpVT && PT) {
+        auto Cmp1 = SDValue(DAG.getMachineNode(XorOp, DL, VecVT, A, B), 0);
+        auto Cmp2 = SDValue(DAG.getMachineNode(XorOp, DL, VecVT, C, D), 0);
----------------
Why are these machine opcodes and not ISD::XOR?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:42430
+      auto PT = DAG.getNode(X86ISD::PTEST, DL, MVT::i32, BCCmp, BCCmp);
+      return DAG.getSetCC(DL, VT, PT, DAG.getConstant(0, DL, MVT::i32), CC);
+    }
----------------
PTEST returns an i32 representing EFLAGS. You can't pass it to ISD::SETCC it doesn't mean anything. The DAG.getSetcc call needs to call the X86ISelLowering.cpp copy of getSetcc that creates an X86ISD::SETCC. You'll need to pass it the X86::COND_E/X86::COND_NE here. This will return an MVT::i8 so you'll need to emit an ISD::TRUNCATE to VT after it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68632