[PATCH] D50233: [InstCombine] Transform str(n)cmp to memcmp

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 3 12:23:11 PDT 2018


efriedma added a comment.

Please fix the tests so they don't strcmp against uninitialized memory.  Probably the simplest way to do that is use an argument instead of an alloca.



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:155
+  if (!isOnlyUsedInZeroEqualityComparison(CI))
+    return false;
+
----------------
Not sure why this check is relevant; memcmp should return the same value as strcmp.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:164
+  if (ArrSize <= Len)
+    return false;
+  return true;
----------------
I don't see how this proves anything... just because you have a pointer of type `[N x i8] *` doesn't imply it's actually backed by N bytes of dereferenceable memory.  Please just use llvm::isDereferenceableAndAlignedPointer.


https://reviews.llvm.org/D50233





More information about the llvm-commits mailing list