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

[Siva Chandra via Phabricator via libc-commits]

sivachandra accepted this revision.
sivachandra added a comment.
This revision is now accepted and ready to land.

OK after addressing the comments which are mostly nits.



================
Comment at: libc/cmake/modules/LLVMLibCCommonTuners.cmake:1
+# ------------------------------------------------------------------------------
+# Common tuning option definitions.
----------------
This file is not a CMake infrastructure component so it should live in the `libc` directory and can have a simple name like `common_libc_tuners.cmake`.


================
Comment at: libc/cmake/modules/LLVMLibCCommonTuners.cmake:7
+
+option(LIBC_STRING_WIDE_READ "Read strings for strlen and memchr as int sized blocks instead of bytes. This relies on undefined behavior and may fail on some systems, but improves performance on long strings." OFF)
+if(LIBC_STRING_WIDE_READ)
----------------
The earlier name of `LIBC_UNSAFE_WORD_READ` was more suggestive than this name. Also, I think the description should be more generic instead of referring to strlen and memchr.


================
Comment at: libc/src/string/string_utils.h:90
+  // performs better than larger sizes when the src pointer can't be assumed to
+  // be aligned to a word boundary, so it's the size I use for reading the
+  // string a block at a time.
----------------
s/`I`/`we`


================
Comment at: libc/src/string/string_utils.h:149
+  // should be in that range.
+  if (max_strlen > (sizeof(unsigned int) * 4)) {
+    // Unsigned int is used for the same reason as in strlen.
----------------
Nit: I think this `unsigned int` is to be in sync with the template `unsigned int` argument. So, define a local type say, `using BlockType = unsigned int;`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129808