[libc-commits] [PATCH] D129808: [libc] add unsafe mode to strlen

[Nick Desaulniers via Phabricator via libc-commits]

nickdesaulniers added a comment.

In D129808#3957855 <https://reviews.llvm.org/D129808#3957855>, @michaelrj wrote:

> add unsafe version to memchr
> move to using just one set of compile flags

Thanks! How about `strchr`? Looks like there's a TODO about this in libc/src/string/strchr.cpp?

For fun, can we do anything like this for `strrchr`? (We're on to the point of diminishing returns here; I only truly care about `strlen` here).

In D129808#3955420 <https://reviews.llvm.org/D129808#3955420>, @sivachandra wrote:

> Also, we will want the default behavior to be the opposite. As in, it should be `OFF` by default and should be switched `ON` explicitly.

So slower by default? :( Not my circus, not my monkeys; but I'll take an implementation over no implementation.



================
Comment at: libc/CMakeLists.txt:36
 
+option(LIBC_FAST_UNSAFE_WORD_READ "Use the faster strlen algorithm that depends on undefined behavior, and may fail on some systems." OFF)
+if(LIBC_FAST_UNSAFE_WORD_READ)
----------------
is `strlen` here still precise after Diff 478668?


================
Comment at: libc/src/string/string_utils.h:87
+#ifdef LIBC_FAST_UNSAFE_WORD_READ
+  // TODO: pick size of var based on requested size.
+  return string_length_unsafe<uintptr_t>(src);
----------------
What requested size? I don't follow what this comment means. Can you expand on it, inline?


================
Comment at: libc/src/string/string_utils.h:139
+#ifdef LIBC_FAST_UNSAFE_WORD_READ
+  // TODO: pick size of var based on requested size.
+  return find_first_character_unsafe<uintptr_t>(src, ch, n);
----------------
Is `n` the "requested size"? If so, care to break out the crayons and be super explicit in the comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129808