| Message ID | 20211021223850.415607-2-goldstein.w.n@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v8,1/3] String: Add support for __memcmpeq() ABI on all targets | expand |
On Thu, Oct 21, 2021 at 3:39 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > No bug. > > This commit adds hidden defs for all declarations of __memcmpeq. This > enables usage of __memcmpeq without the PLT for usage internal to > GLIBC. > --- > include/string.h | 1 + > string/memcmp.c | 1 + > sysdeps/aarch64/memcmp.S | 1 + > sysdeps/csky/abiv2/memcmp.S | 1 + > sysdeps/i386/i686/memcmp.S | 1 + > sysdeps/i386/i686/multiarch/memcmp.c | 1 + > sysdeps/i386/memcmp.S | 1 + > sysdeps/ia64/memcmp.S | 1 + > sysdeps/powerpc/powerpc32/405/memcmp.S | 1 + > sysdeps/powerpc/powerpc32/power4/memcmp.S | 1 + > sysdeps/powerpc/powerpc32/power7/memcmp.S | 1 + > sysdeps/powerpc/powerpc64/le/power10/memcmp.S | 1 + > sysdeps/powerpc/powerpc64/power4/memcmp.S | 1 + > sysdeps/powerpc/powerpc64/power7/memcmp.S | 1 + > sysdeps/powerpc/powerpc64/power8/memcmp.S | 1 + > sysdeps/s390/memcmp-z900.S | 1 + > sysdeps/s390/memcmp.c | 1 + > sysdeps/sparc/sparc64/memcmp.S | 1 + > sysdeps/x86_64/memcmp.S | 1 + > sysdeps/x86_64/multiarch/memcmp.c | 1 + > 20 files changed, 20 insertions(+) > > diff --git a/include/string.h b/include/string.h > index 81dab39891..21f641a413 100644 > --- a/include/string.h > +++ b/include/string.h > @@ -112,6 +112,7 @@ extern char *__strsep_g (char **__stringp, const char *__delim); > libc_hidden_proto (__strsep_g) > libc_hidden_proto (strnlen) > libc_hidden_proto (__strnlen) > +libc_hidden_proto (__memcmpeq) > libc_hidden_proto (memmem) > extern __typeof (memmem) __memmem; > libc_hidden_proto (__memmem) There are many memcmp calls in ld.so. I think most, if not all, of them can be changed to __memcmpeq. Can you make another patch to do that?
On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > There are many memcmp calls in ld.so. I think most, if not all, of them > can be changed to __memcmpeq. Can you make another patch to do > that? Rather than doing that micro-optimization in the glibc sources, I think it's better to add the relevant feature to GCC and let glibc get optimized when built with a new-enough compiler.
On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote: > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > > > There are many memcmp calls in ld.so. I think most, if not all, of them > > can be changed to __memcmpeq. Can you make another patch to do > > that? > > Rather than doing that micro-optimization in the glibc sources, I think > it's better to add the relevant feature to GCC and let glibc get optimized > when built with a new-enough compiler. > Why not? We don't know when __memcmpeq will be supported by GCC used by glibc developers. If we don't even use it in glibc, why bother? -- H.J.
On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > > > > > There are many memcmp calls in ld.so. I think most, if not all, of them > > > can be changed to __memcmpeq. Can you make another patch to do > > > that? > > > > Rather than doing that micro-optimization in the glibc sources, I think > > it's better to add the relevant feature to GCC and let glibc get optimized > > when built with a new-enough compiler. > > Why not? We don't know when __memcmpeq will be supported by GCC > used by glibc developers. If we don't even use it in glibc, why bother? The point of this function is entirely for compilers to generate for calls from memcmp, not for humans to write direct calls to. The compiler knows other semantics of memcmp (it might sometimes expand a memcmp with a small fixed size inline, for example). So changing to a direct call to __memcmpeq by hand isn't even always an optimization; it seems better to leave the semantics visible to the compiler. Note that in other cases we've generally moved *towards* relying on built-in functions rather than having our own local inline expansions of those or calls to internal names for those functions (we've done that for calls to various functions in libm, for example).
On Thu, Oct 21, 2021 at 6:15 PM Joseph Myers <joseph@codesourcery.com> wrote: > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > > > On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > > > > > > > There are many memcmp calls in ld.so. I think most, if not all, of them > > > > can be changed to __memcmpeq. Can you make another patch to do > > > > that? > > > > > > Rather than doing that micro-optimization in the glibc sources, I think > > > it's better to add the relevant feature to GCC and let glibc get optimized > > > when built with a new-enough compiler. > > > > Why not? We don't know when __memcmpeq will be supported by GCC > > used by glibc developers. If we don't even use it in glibc, why bother? > > The point of this function is entirely for compilers to generate for calls > from memcmp, not for humans to write direct calls to. > > The compiler knows other semantics of memcmp (it might sometimes expand a > memcmp with a small fixed size inline, for example). So changing to a > direct call to __memcmpeq by hand isn't even always an optimization; it > seems better to leave the semantics visible to the compiler. Note that in > other cases we've generally moved *towards* relying on built-in functions > rather than having our own local inline expansions of those or calls to > internal names for those functions (we've done that for calls to various > functions in libm, for example). I have gone through all non-test .c files in GLIBC and replaced memcmp with __memcmpeq where it makes sense. I can post that as a separate patchset and we can decide whether it's a good idea there. > > -- > Joseph S. Myers > joseph@codesourcery.com
On Thu, Oct 21, 2021 at 6:39 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote: > > On Thu, Oct 21, 2021 at 6:15 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > > > > > On Thu, Oct 21, 2021 at 4:08 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > > > > > On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > > > > > > > > > There are many memcmp calls in ld.so. I think most, if not all, of them > > > > > can be changed to __memcmpeq. Can you make another patch to do > > > > > that? > > > > > > > > Rather than doing that micro-optimization in the glibc sources, I think > > > > it's better to add the relevant feature to GCC and let glibc get optimized > > > > when built with a new-enough compiler. > > > > > > Why not? We don't know when __memcmpeq will be supported by GCC > > > used by glibc developers. If we don't even use it in glibc, why bother? > > > > The point of this function is entirely for compilers to generate for calls > > from memcmp, not for humans to write direct calls to. > > > > The compiler knows other semantics of memcmp (it might sometimes expand a > > memcmp with a small fixed size inline, for example). So changing to a > > direct call to __memcmpeq by hand isn't even always an optimization; Will compilers not implement the same optimization for __memcmpeq? > it > > seems better to leave the semantics visible to the compiler. Note that in > > other cases we've generally moved *towards* relying on built-in functions > > rather than having our own local inline expansions of those or calls to > > internal names for those functions (we've done that for calls to various > > functions in libm, for example). > > I have gone through all non-test .c files in GLIBC and replaced memcmp > with __memcmpeq where it makes sense. I can post that as a separate > patchset and we can decide whether it's a good idea there. > > > > > -- > > Joseph S. Myers > > joseph@codesourcery.com
On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote: > I have gone through all non-test .c files in GLIBC and replaced memcmp > with __memcmpeq where it makes sense. I can post that as a separate > patchset and we can decide whether it's a good idea there. I think it's a bad idea - it obfuscates the source code and it's better just to call the standard function, leave the code more readable and let the compiler optimize it as appropriate.
On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote: > > > The compiler knows other semantics of memcmp (it might sometimes expand a > > > memcmp with a small fixed size inline, for example). So changing to a > > > direct call to __memcmpeq by hand isn't even always an optimization; > Will compilers not implement the same optimization for __memcmpeq? Why should they? It's intended as an ABI, not an API - that is, an alternative assembler name the compiler name generate calls to, not a function for which it should find calls in its input.
On Thu, Oct 21, 2021 at 5:19 PM Joseph Myers <joseph@codesourcery.com> wrote: > > On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote: > > > > > The compiler knows other semantics of memcmp (it might sometimes expand a > > > > memcmp with a small fixed size inline, for example). So changing to a > > > > direct call to __memcmpeq by hand isn't even always an optimization; > > Will compilers not implement the same optimization for __memcmpeq? > > Why should they? It's intended as an ABI, not an API - that is, an > alternative assembler name the compiler name generate calls to, not a > function for which it should find calls in its input. > Compiler builtin optimization doesn't work well for glibc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67220
On Thu, 21 Oct 2021, H.J. Lu via Libc-alpha wrote: > On Thu, Oct 21, 2021 at 5:19 PM Joseph Myers <joseph@codesourcery.com> wrote: > > > > On Thu, 21 Oct 2021, Noah Goldstein via Libc-alpha wrote: > > > > > > > The compiler knows other semantics of memcmp (it might sometimes expand a > > > > > memcmp with a small fixed size inline, for example). So changing to a > > > > > direct call to __memcmpeq by hand isn't even always an optimization; > > > Will compilers not implement the same optimization for __memcmpeq? > > > > Why should they? It's intended as an ABI, not an API - that is, an > > alternative assembler name the compiler name generate calls to, not a > > function for which it should find calls in its input. > > > > Compiler builtin optimization doesn't work well for glibc: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67220 1. So let's fix that in the compiler. This doesn't seem like something so hard to fix that it should justify obfuscating the source code in glibc by putting in unnecessary direct calls to an internal name. 2. If we need a single redirection of __memcmpeq to the hidden alias __GI___memcmpeq, we could certainly put that in sysdeps/generic/symbol-hacks.h as we do for a few other functions. A redirection like that in one place isn't problematic the same way putting in lots of direct calls to an internal name in the source code (for something better optimized in the compiler) is.
diff --git a/include/string.h b/include/string.h index 81dab39891..21f641a413 100644 --- a/include/string.h +++ b/include/string.h @@ -112,6 +112,7 @@ extern char *__strsep_g (char **__stringp, const char *__delim); libc_hidden_proto (__strsep_g) libc_hidden_proto (strnlen) libc_hidden_proto (__strnlen) +libc_hidden_proto (__memcmpeq) libc_hidden_proto (memmem) extern __typeof (memmem) __memmem; libc_hidden_proto (__memmem) diff --git a/string/memcmp.c b/string/memcmp.c index 5020be00e0..ecffc1d34a 100644 --- a/string/memcmp.c +++ b/string/memcmp.c @@ -360,4 +360,5 @@ libc_hidden_builtin_def(memcmp) weak_alias (memcmp, bcmp) # undef __memcmpeq weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) #endif diff --git a/sysdeps/aarch64/memcmp.S b/sysdeps/aarch64/memcmp.S index bc932eff2a..318124a4fd 100644 --- a/sysdeps/aarch64/memcmp.S +++ b/sysdeps/aarch64/memcmp.S @@ -180,3 +180,4 @@ weak_alias (memcmp, bcmp) #undef __memcmpeq weak_alias (memcmp, __memcmpeq) libc_hidden_builtin_def (memcmp) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/csky/abiv2/memcmp.S b/sysdeps/csky/abiv2/memcmp.S index d61fca0f29..f4179769d2 100644 --- a/sysdeps/csky/abiv2/memcmp.S +++ b/sysdeps/csky/abiv2/memcmp.S @@ -140,4 +140,5 @@ END (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) libc_hidden_def (memcmp) +libc_hidden_def (__memcmpeq) .weak memcmp diff --git a/sysdeps/i386/i686/memcmp.S b/sysdeps/i386/i686/memcmp.S index 0194f8deab..b61fba2be1 100644 --- a/sysdeps/i386/i686/memcmp.S +++ b/sysdeps/i386/i686/memcmp.S @@ -408,3 +408,4 @@ weak_alias (memcmp, bcmp) #undef __memcmpeq weak_alias (memcmp, __memcmpeq) libc_hidden_builtin_def (memcmp) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/i386/i686/multiarch/memcmp.c b/sysdeps/i386/i686/multiarch/memcmp.c index 956fc8e601..575471f1ec 100644 --- a/sysdeps/i386/i686/multiarch/memcmp.c +++ b/sysdeps/i386/i686/multiarch/memcmp.c @@ -30,4 +30,5 @@ libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ()); weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) #endif diff --git a/sysdeps/i386/memcmp.S b/sysdeps/i386/memcmp.S index 18e225f963..2802da7833 100644 --- a/sysdeps/i386/memcmp.S +++ b/sysdeps/i386/memcmp.S @@ -73,3 +73,4 @@ weak_alias (memcmp, bcmp) #undef __memcmpeq weak_alias (memcmp, __memcmpeq) libc_hidden_builtin_def (memcmp) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/ia64/memcmp.S b/sysdeps/ia64/memcmp.S index 4488e0eba4..ae4be49a64 100644 --- a/sysdeps/ia64/memcmp.S +++ b/sysdeps/ia64/memcmp.S @@ -162,3 +162,4 @@ END(memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) libc_hidden_builtin_def (memcmp) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc32/405/memcmp.S b/sysdeps/powerpc/powerpc32/405/memcmp.S index e8b1b6c9bd..61949db62f 100644 --- a/sysdeps/powerpc/powerpc32/405/memcmp.S +++ b/sysdeps/powerpc/powerpc32/405/memcmp.S @@ -127,3 +127,4 @@ END (memcmp) libc_hidden_builtin_def (memcmp) weak_alias (memcmp,bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc32/power4/memcmp.S b/sysdeps/powerpc/powerpc32/power4/memcmp.S index e4dde875bb..e7e2ab13b5 100644 --- a/sysdeps/powerpc/powerpc32/power4/memcmp.S +++ b/sysdeps/powerpc/powerpc32/power4/memcmp.S @@ -1374,3 +1374,4 @@ END (memcmp) libc_hidden_builtin_def (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc32/power7/memcmp.S b/sysdeps/powerpc/powerpc32/power7/memcmp.S index e60a62fc86..2047f70e82 100644 --- a/sysdeps/powerpc/powerpc32/power7/memcmp.S +++ b/sysdeps/powerpc/powerpc32/power7/memcmp.S @@ -1374,3 +1374,4 @@ END (memcmp) libc_hidden_builtin_def (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc64/le/power10/memcmp.S b/sysdeps/powerpc/powerpc64/le/power10/memcmp.S index c7fe8047ca..ed682a6303 100644 --- a/sysdeps/powerpc/powerpc64/le/power10/memcmp.S +++ b/sysdeps/powerpc/powerpc64/le/power10/memcmp.S @@ -178,3 +178,4 @@ END (MEMCMP) libc_hidden_builtin_def (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc64/power4/memcmp.S b/sysdeps/powerpc/powerpc64/power4/memcmp.S index 4ab4a90496..01cbed80df 100644 --- a/sysdeps/powerpc/powerpc64/power4/memcmp.S +++ b/sysdeps/powerpc/powerpc64/power4/memcmp.S @@ -1375,3 +1375,4 @@ END (MEMCMP) libc_hidden_builtin_def (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc64/power7/memcmp.S b/sysdeps/powerpc/powerpc64/power7/memcmp.S index b541978b5f..aaf1a36263 100644 --- a/sysdeps/powerpc/powerpc64/power7/memcmp.S +++ b/sysdeps/powerpc/powerpc64/power7/memcmp.S @@ -1060,3 +1060,4 @@ END (MEMCMP) libc_hidden_builtin_def (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/powerpc/powerpc64/power8/memcmp.S b/sysdeps/powerpc/powerpc64/power8/memcmp.S index d4f84a5b6f..520c31a6b1 100644 --- a/sysdeps/powerpc/powerpc64/power8/memcmp.S +++ b/sysdeps/powerpc/powerpc64/power8/memcmp.S @@ -1443,3 +1443,4 @@ END (MEMCMP) libc_hidden_builtin_def (memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/s390/memcmp-z900.S b/sysdeps/s390/memcmp-z900.S index 0942d32814..c10fdb27fa 100644 --- a/sysdeps/s390/memcmp-z900.S +++ b/sysdeps/s390/memcmp-z900.S @@ -165,6 +165,7 @@ END(MEMCMP_Z196) strong_alias (MEMCMP_DEFAULT, memcmp) weak_alias (memcmp, bcmp) weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) #endif #if defined SHARED && IS_IN (libc) diff --git a/sysdeps/s390/memcmp.c b/sysdeps/s390/memcmp.c index 475fc45d3a..c381e038e2 100644 --- a/sysdeps/s390/memcmp.c +++ b/sysdeps/s390/memcmp.c @@ -47,4 +47,5 @@ s390_libc_ifunc_expr (__redirect_memcmp, memcmp, ) weak_alias (memcmp, bcmp); weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) #endif diff --git a/sysdeps/sparc/sparc64/memcmp.S b/sysdeps/sparc/sparc64/memcmp.S index edcc19915a..7ecee3281f 100644 --- a/sysdeps/sparc/sparc64/memcmp.S +++ b/sysdeps/sparc/sparc64/memcmp.S @@ -140,3 +140,4 @@ weak_alias (memcmp, bcmp) #undef __memcmpeq weak_alias (memcmp, __memcmpeq) libc_hidden_builtin_def (memcmp) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S index f41ae48006..5f4a09d5a4 100644 --- a/sysdeps/x86_64/memcmp.S +++ b/sysdeps/x86_64/memcmp.S @@ -361,3 +361,4 @@ weak_alias (memcmp, bcmp) #undef __memcmpeq weak_alias (memcmp, __memcmpeq) libc_hidden_builtin_def (memcmp) +libc_hidden_def (__memcmpeq) diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c index fe125e0904..427a1a9ede 100644 --- a/sysdeps/x86_64/multiarch/memcmp.c +++ b/sysdeps/x86_64/multiarch/memcmp.c @@ -31,6 +31,7 @@ libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ()); weak_alias (memcmp, bcmp) # undef __memcmpeq weak_alias (memcmp, __memcmpeq) +libc_hidden_def (__memcmpeq) # ifdef SHARED __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)