[libc-commits] [PATCH] D106511: [libc][NFC] Optimize and standardize the C string functions

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

One way is to fold this entire patch "Under standardize and optimize" but I would rather split this patch to actually see the "standardize" parts and the "optimize" parts separately.  In fact, as it stands, this patch is doing //cleanups// beyond "optimize" and "standardize". So, I would strongly encourage you to separate out all the individual concerns clearly in different patches.



================
Comment at: libc/src/string/memmove.cpp:1
 //===-- Implementation of memmove -----------------------------------------===//
 //
----------------
Some of the changes in this file have to separated out into a patch of their own.


================
Comment at: libc/src/string/memory_utils/elements.h:1
 //===-- Elementary operations to compose memory primitives ----------------===//
 //
----------------
Same here - some of the changes have to be a patch of their own explaining the rationale clearly. You can combine this change with the change to `memmove.cpp`.


================
Comment at: libc/src/string/memrchr.cpp:19
   for (; n != 0; --n) {
-    const unsigned char *s = str + n - 1;
-    if (*s == ch)
-      return const_cast<unsigned char *>(s);
+    if (*(--str) == ch)
+      return const_cast<unsigned char *>(str);
----------------
gAlfonso-bit wrote:
> sivachandra wrote:
> > Instead of folding existing code into patterns like this (using an increment and decrement operator along with a another operator like the dereference operator), I would suggest doing the other way round cleanup. For example, the existing code is more readable than this new code.
> The code is more efficient this way than the last way.
Can you please demonstrate that at `-O2` or higher, mixing an increment/decrement operator with another operator is more efficient? 


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

https://reviews.llvm.org/D106511