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

Simon Pilgrim via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 10 07:36:00 PDT 2019


RKSimon added a comment.

In general we don't use auto for anything other than casts or iterators - see the llvm coding style guidelines



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:42374
   SDLoc DL(SetCC);
+  auto AVX = Subtarget.hasAVX();
+
----------------
bool HasAVX = Subtarget.hasAVX();


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:42385
+                PT ? (OpSize == 256 ? MVT::v8f32 : MVT::v4f32) :
+                OpSize == 256 ? MVT::v32i8 : MVT::v16i8;
     EVT CmpVT = OpSize == 512 ? (BW ? MVT::v64i1 : MVT::v16i1) : VecVT;
----------------
I'd prefer not to nest ternary operators like this - either nest them in brackets or if-else


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:42390
+                           AVX ? X86::VXORPSrr : X86::XORPSrr;
+    auto OrOp = OpSize == 256 ? X86::VORPSYrr :
+                          AVX ? X86::VORPSrr : X86::ORPSrr;
----------------
(style guide) Don't use auto - use unsigned


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68632





More information about the llvm-commits mailing list