[libc-commits] [PATCH] D72516: [llvm-libc] Add memory function benchmarks

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/utils/benchmarks/Memcmp.cpp:64
+    FunctionPrototype Function =
+        StringSwitch<FunctionPrototype>(FunctionName).Case("memcmp", &::memcmp);
+    return llvm::libc_benchmarks::benchmark(
----------------
MaskRay wrote:
> abrachet wrote:
> > sivachandra wrote:
> > > ckennelly wrote:
> > > > Should we setup an analogous benchmark for `bcmp`?
> > > Shoud llvm-libc provide bcmp at all?
> > In what I believe was @gchatelet's email on the list said they were hoping `!memcmp(...)` would be emitted as `!bcmp(...)` for efficiency. [[ https://godbolt.org/z/5zBm4k | clang ]] does make this optimization.
> TargetLibraryInfo.cpp
> 
> ```
> static bool hasBcmp(const Triple &TT) {
>   // Posix removed support from bcmp() in 2001, but the glibc and several
>   // implementations of the libc still have it.
>   if (TT.isOSLinux())
>     return TT.isGNUEnvironment() || TT.isMusl();
>   // Both NetBSD and OpenBSD are planning to remove the function. Windows does
>   // not have it.
>   return TT.isOSFreeBSD() || TT.isOSSolaris();
> }
> ```
> 
> On certain platforms (including Linux), memcmp can be optimized to bcmp if preferable.
To be honest, I do not have any opinion. If others more knowledgeable in this area decide llvm-libc should have a `bcmp`, then we will go with it.

Just pointing out though: musl's `bcmp` just calls into `memcmp`, and in glibc, `bcmp` is an alias of `memcmp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72516