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

[David Zarzycki via Phabricator via llvm-commits]

davezarzycki marked 2 inline comments as done.
davezarzycki added inline comments.


================
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);
----------------
craig.topper wrote:
> Why are these machine opcodes and not ISD::XOR?
My first attempt at this patch used ISD::XOR after the vector bitcasts, but scalar instructions were often (but not always) were emitted. I later noticed the comment at the top of the function which suggests/implies that explicit vector instructions are the right solution and indeed that worked reliably.


================
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);
+    }
----------------
craig.topper wrote:
> 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.
Thanks for the PTEST feedback. I think I have it working the way you describe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68632