<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jan 7, 2019 at 10:26 PM James Y Knight <<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Mon, Jan 7, 2019 at 5:50 AM Clement Courbet <<a href="mailto:courbet@google.com" target="_blank">courbet@google.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div dir="ltr">On Fri, Jan 4, 2019 at 12:34 PM James Y Knight via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">This seems a somewhat odd and overcomplicated way to go about this.<div><br></div><div>Given that bcmp was required in POSIX until relatively recently, I will guess that almost all platforms support it already. From a quick check, glibc, freebsd, netbsd, newlib, and musl all seem to contain it. So, couldn't we just add bcmp to the runtime function list for those platforms which support it? And, add an optimization to translate a call to memcmp into bcmp if it exists?</div></div></blockquote></div></blockquote><div><br></div><div>That would indeed be much simpler, but this seems brittle to me. The approach you're suggesting works for us (google) because we fully control our environment, but I'm afraid it will not work for others. </div><div>For example, someone might distribute linux binaries built with LLVM to their users, but since there is nothing guaranteeing that bcmp is available on the user's libc, they won't be able to run it on their systems.</div><div>Is there a precedent for that approach ?</div></div></div></blockquote><div><br></div><div>There are many library functions that are only available on some platforms and not others. LLVM can easily be told which do include it and which do not, and emit a call only for those which do.</div><div><br></div><div>For Glibc Linux, we can be sure that bcmp is available on all systems -- is is there now, it always has been, and always will be in the future (due to backwards compatibility guarantees). It's just defined as an alias for memcmp, however, so there's no advantage (nor, for that matter, disadvantage) to using it.</div><div><br></div><div>But of course it's not _only_ glibc which has it -- it's present in almost every environment out there.</div><div><br></div><div>e.g. for Linux there's also a few other libc implementations that people use:</div><div>musl: includes it, calls memcmp.</div><div>uclibc-ng: includes it, alias of memcmp (only if you compile with UCLIBC_SUSV3_LEGACY, but "buildroot", the way one generally uses uclibc, does so).</div></div></div></div></blockquote><div><br></div><div>I'm afraid about the "almost" and "generally": what about users who don't ?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div>NetBSD: includes it, separate optimized implementation.</div></div></div></div></blockquote><div><br></div><div>Quoting from the NetBSD <a href="http://netbsd.gw.com/cgi-bin/man-cgi?bcmp+9+NetBSD-current" target="_blank">man page</a>:</div><div>"The bcmp() interface is obsolete.  Do not add new code using it.  It will soon be purged.  Use memcmp(9) instead.  (The bcmp() function is now a  macro for memcmp(9).)"</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div>I do note, sadly, that currently out of all these implementations, only NetBSD and FreeBSD seem to actually define a separate more optimized bcmp function. That does mean that this optimization would be effectively a no-op, for the vast majority of people.<br></div></div></div></div></blockquote><div><br></div><div>This might or might not be considered really an issue.</div><div> - In my original proposal, people have to explicitly opt-in to the feature and link to their memcmp implementation, they do not get the improvement automatically. </div><div> - In this proposal, they have to patch their libc, which might be slightly more painful depending on the system.</div><div><br></div><div>Here's a patch with this proposal to see what this looks like: <a href="https://reviews.llvm.org/D56436">https://reviews.llvm.org/D56436</a></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div>For Google's purposes, if a call to bcmp is emitted by the compiler, you can of course just override it with a more optimal version.</div></div></div></div></blockquote><div><br></div><div>For our purpose anything is fine because we fully control our environment. I'm trying to make sure that we do not create pain for other users.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div> But if you can show a similar performance win in public code, it'd be great to attempt to push a more optimized version upstream at least to glibc. Some more precise numbers than "very large improvement" are probably necessary to show it's actually worth it. :)</div></div></div></div></blockquote><div><br></div><div>We were planning to contribute it to compiler-rt. Contributing a deprecated function to the libc sounds.... weird.</div><div> </div></div></div></div></div></div>