[libc-commits] [PATCH] D82134: [libc] Add strcmp implementation.

[Guillaume Chatelet via Phabricator via libc-commits]

gchatelet added inline comments.


================
Comment at: libc/src/string/strcmp.cpp:15
+
+int LLVM_LIBC_ENTRYPOINT(strcmp)(const char *l, const char *r) {
+  while (*l) {
----------------
cgyurgyik wrote:
> PaulkaToast wrote:
> > sivachandra wrote:
> > > Is there benefit in comparing words at a time? I know that it leads to potential OOB reads, so we can handle it separately. But if it can lead to speed up, just a leave a TODO about it and we will get to it in a later round.
> > I would suggest to opt for more descriptive variable names over single letter one. i.e ("left" instead of "l").
> What is the standard for leaving TODOs? I'm not finding anything in the llvm Coding Standards.
> Is there benefit in comparing words at a time? I know that it leads to potential OOB reads, so we can handle it separately. But if it can lead to speed up, just a leave a TODO about it and we will get to it in a later round.

Technically we have to read the buffers byte per byte as we're not supposed to read past the `\0`.
I'm not optimistic on the ability to accelerate this routine because -even if a page fault can only occur at page boundaries- some processors are offering [[ https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/Learn%20the%20Architecture/Providing%20protection%20for%20complex%20software.pdf | hardware pointer authentication ]] which can break reading buffers cache line per cache line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82134