[libc-commits] [PATCH] D83353: [libc] Add strchr implementation.

Fri Jul 10 10:46:39 PDT 2020

cgyurgyik added a comment.

In D83353#2144598 <https://reviews.llvm.org/D83353#2144598>, @sivachandra wrote:

> Since the scope of this change has widened, please update the commit message accordingly. Also, if you are not using arcanist, add the link to the phabricator review page in the commit message. You will have to do this on this review page as well as in your local commit.

Done.

================
Comment at: libc/src/string/strchr.cpp:19
+  char *str = const_cast<char *>(src);
+  if (!c)
+    return str + __llvm_libc::strlen(str);
----------------
sivachandra wrote:
> cgyurgyik wrote:
> > sivachandra wrote:
> > > Whats the benefit of doing this check? Wouldn't the return statement already handle this case?
> > It would, but using strlen requires only a single comparison with `0` while the loop below requires a comparison with `0` and a comparison with `ch`.
> I would expect that a much higher percentage of calls to `strchr` would be with a non-zero `c`.  In which case, this seems like an optimization for a very special case. For the higher percentage case, it would be an additional conditional. 
Removed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83353