* [PATCH] x86-64: Make bcmp an alias of __memcmpeq
@ 2022-02-06 19:26 H.J. Lu
2022-02-06 19:34 ` Noah Goldstein
0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2022-02-06 19:26 UTC (permalink / raw)
To: libc-alpha
---
sysdeps/x86_64/memcmp.S | 2 +-
sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
sysdeps/x86_64/multiarch/memcmp.c | 2 --
sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
4 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
index e02a53ea1e..2768d45f10 100644
--- a/sysdeps/x86_64/memcmp.S
+++ b/sysdeps/x86_64/memcmp.S
@@ -405,8 +405,8 @@ END(memcmp)
#ifdef USE_AS_MEMCMPEQ
libc_hidden_def (memcmp)
-#else
# undef bcmp
weak_alias (memcmp, bcmp)
+#else
libc_hidden_builtin_def (memcmp)
#endif
diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
index e10555638d..d825c13ede 100644
--- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
+++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
@@ -29,8 +29,10 @@
# define libc_hidden_def(ignored)
# endif
-# undef weak_alias
-# define weak_alias(ignored1, ignored2)
+# ifdef USE_MULTIARCH
+# undef weak_alias
+# define weak_alias(ignored1, ignored2)
+# endif
# undef strong_alias
# define strong_alias(ignored1, ignored2)
diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
index 7757d1ec4e..bae29d8dad 100644
--- a/sysdeps/x86_64/multiarch/memcmp.c
+++ b/sysdeps/x86_64/multiarch/memcmp.c
@@ -27,8 +27,6 @@
# include "ifunc-memcmp.h"
libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
-# undef bcmp
-weak_alias (memcmp, bcmp)
# ifdef SHARED
__hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
index aa1c8ad4de..45cb6bad5b 100644
--- a/sysdeps/x86_64/multiarch/memcmpeq.c
+++ b/sysdeps/x86_64/multiarch/memcmpeq.c
@@ -27,6 +27,8 @@
# include "ifunc-memcmpeq.h"
libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
+# undef bcmp
+weak_alias (__memcmpeq, bcmp)
# ifdef SHARED
__hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86-64: Make bcmp an alias of __memcmpeq
2022-02-06 19:26 [PATCH] x86-64: Make bcmp an alias of __memcmpeq H.J. Lu
@ 2022-02-06 19:34 ` Noah Goldstein
2022-02-06 19:57 ` H.J. Lu
0 siblings, 1 reply; 4+ messages in thread
From: Noah Goldstein @ 2022-02-06 19:34 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On Sun, Feb 6, 2022 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> ---
> sysdeps/x86_64/memcmp.S | 2 +-
> sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
> sysdeps/x86_64/multiarch/memcmp.c | 2 --
> sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
> 4 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
> index e02a53ea1e..2768d45f10 100644
> --- a/sysdeps/x86_64/memcmp.S
> +++ b/sysdeps/x86_64/memcmp.S
> @@ -405,8 +405,8 @@ END(memcmp)
>
> #ifdef USE_AS_MEMCMPEQ
> libc_hidden_def (memcmp)
> -#else
> # undef bcmp
> weak_alias (memcmp, bcmp)
> +#else
> libc_hidden_builtin_def (memcmp)
> #endif
> diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> index e10555638d..d825c13ede 100644
> --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
> +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> @@ -29,8 +29,10 @@
> # define libc_hidden_def(ignored)
> # endif
>
> -# undef weak_alias
> -# define weak_alias(ignored1, ignored2)
> +# ifdef USE_MULTIARCH
> +# undef weak_alias
> +# define weak_alias(ignored1, ignored2)
> +# endif
>
> # undef strong_alias
> # define strong_alias(ignored1, ignored2)
> diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
> index 7757d1ec4e..bae29d8dad 100644
> --- a/sysdeps/x86_64/multiarch/memcmp.c
> +++ b/sysdeps/x86_64/multiarch/memcmp.c
> @@ -27,8 +27,6 @@
> # include "ifunc-memcmp.h"
>
> libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
> -# undef bcmp
> -weak_alias (memcmp, bcmp)
>
> # ifdef SHARED
> __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
> diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
> index aa1c8ad4de..45cb6bad5b 100644
> --- a/sysdeps/x86_64/multiarch/memcmpeq.c
> +++ b/sysdeps/x86_64/multiarch/memcmpeq.c
> @@ -27,6 +27,8 @@
> # include "ifunc-memcmpeq.h"
>
> libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
> +# undef bcmp
> +weak_alias (__memcmpeq, bcmp)
>
> # ifdef SHARED
> __hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
> --
> 2.34.1
>
Are we sure this is safe? One of the original rationales for adding
'__memcmpeq' as
a new interface instead of just optimizing 'bcmp' was that users may
be relying on
the fact that 'bcmp' has historically returned the LT/GT of the comparison.
As well, if we are going to change 'bcmp' should we do it for all
architectures so that
its consistent?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86-64: Make bcmp an alias of __memcmpeq
2022-02-06 19:34 ` Noah Goldstein
@ 2022-02-06 19:57 ` H.J. Lu
2022-02-06 20:16 ` Noah Goldstein
0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2022-02-06 19:57 UTC (permalink / raw)
To: Noah Goldstein; +Cc: GNU C Library
On Sun, Feb 6, 2022 at 11:34 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Sun, Feb 6, 2022 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > ---
> > sysdeps/x86_64/memcmp.S | 2 +-
> > sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
> > sysdeps/x86_64/multiarch/memcmp.c | 2 --
> > sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
> > 4 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
> > index e02a53ea1e..2768d45f10 100644
> > --- a/sysdeps/x86_64/memcmp.S
> > +++ b/sysdeps/x86_64/memcmp.S
> > @@ -405,8 +405,8 @@ END(memcmp)
> >
> > #ifdef USE_AS_MEMCMPEQ
> > libc_hidden_def (memcmp)
> > -#else
> > # undef bcmp
> > weak_alias (memcmp, bcmp)
> > +#else
> > libc_hidden_builtin_def (memcmp)
> > #endif
> > diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > index e10555638d..d825c13ede 100644
> > --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > @@ -29,8 +29,10 @@
> > # define libc_hidden_def(ignored)
> > # endif
> >
> > -# undef weak_alias
> > -# define weak_alias(ignored1, ignored2)
> > +# ifdef USE_MULTIARCH
> > +# undef weak_alias
> > +# define weak_alias(ignored1, ignored2)
> > +# endif
> >
> > # undef strong_alias
> > # define strong_alias(ignored1, ignored2)
> > diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
> > index 7757d1ec4e..bae29d8dad 100644
> > --- a/sysdeps/x86_64/multiarch/memcmp.c
> > +++ b/sysdeps/x86_64/multiarch/memcmp.c
> > @@ -27,8 +27,6 @@
> > # include "ifunc-memcmp.h"
> >
> > libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
> > -# undef bcmp
> > -weak_alias (memcmp, bcmp)
> >
> > # ifdef SHARED
> > __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
> > diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
> > index aa1c8ad4de..45cb6bad5b 100644
> > --- a/sysdeps/x86_64/multiarch/memcmpeq.c
> > +++ b/sysdeps/x86_64/multiarch/memcmpeq.c
> > @@ -27,6 +27,8 @@
> > # include "ifunc-memcmpeq.h"
> >
> > libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
> > +# undef bcmp
> > +weak_alias (__memcmpeq, bcmp)
> >
> > # ifdef SHARED
> > __hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
> > --
> > 2.34.1
> >
>
> Are we sure this is safe? One of the original rationales for adding
> '__memcmpeq' as
> a new interface instead of just optimizing 'bcmp' was that users may
> be relying on
> the fact that 'bcmp' has historically returned the LT/GT of the comparison.
The bcmp man page has:
NAME
bcmp - compare byte sequences
SYNOPSIS
#include <strings.h>
int bcmp(const void *s1, const void *s2, size_t n);
DESCRIPTION
The bcmp() function compares the two byte sequences s1 and s2 of length
n each. If they are equal, and in particular if n is zero, bcmp() re‐
turns 0. Otherwise, it returns a nonzero result.
RETURN VALUE
The bcmp() function returns 0 if the byte sequences are equal, other‐
wise a nonzero result is returned.
> As well, if we are going to change 'bcmp' should we do it for all
> architectures so that
> its consistent?
Only x86-64 has __memcmpeq which isn't an alias of memcmp.
--
H.J.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86-64: Make bcmp an alias of __memcmpeq
2022-02-06 19:57 ` H.J. Lu
@ 2022-02-06 20:16 ` Noah Goldstein
0 siblings, 0 replies; 4+ messages in thread
From: Noah Goldstein @ 2022-02-06 20:16 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On Sun, Feb 6, 2022 at 1:57 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sun, Feb 6, 2022 at 11:34 AM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > On Sun, Feb 6, 2022 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > ---
> > > sysdeps/x86_64/memcmp.S | 2 +-
> > > sysdeps/x86_64/multiarch/memcmp-sse2.S | 6 ++++--
> > > sysdeps/x86_64/multiarch/memcmp.c | 2 --
> > > sysdeps/x86_64/multiarch/memcmpeq.c | 2 ++
> > > 4 files changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/sysdeps/x86_64/memcmp.S b/sysdeps/x86_64/memcmp.S
> > > index e02a53ea1e..2768d45f10 100644
> > > --- a/sysdeps/x86_64/memcmp.S
> > > +++ b/sysdeps/x86_64/memcmp.S
> > > @@ -405,8 +405,8 @@ END(memcmp)
> > >
> > > #ifdef USE_AS_MEMCMPEQ
> > > libc_hidden_def (memcmp)
> > > -#else
> > > # undef bcmp
> > > weak_alias (memcmp, bcmp)
> > > +#else
> > > libc_hidden_builtin_def (memcmp)
> > > #endif
> > > diff --git a/sysdeps/x86_64/multiarch/memcmp-sse2.S b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > > index e10555638d..d825c13ede 100644
> > > --- a/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > > +++ b/sysdeps/x86_64/multiarch/memcmp-sse2.S
> > > @@ -29,8 +29,10 @@
> > > # define libc_hidden_def(ignored)
> > > # endif
> > >
> > > -# undef weak_alias
> > > -# define weak_alias(ignored1, ignored2)
> > > +# ifdef USE_MULTIARCH
> > > +# undef weak_alias
> > > +# define weak_alias(ignored1, ignored2)
> > > +# endif
> > >
> > > # undef strong_alias
> > > # define strong_alias(ignored1, ignored2)
> > > diff --git a/sysdeps/x86_64/multiarch/memcmp.c b/sysdeps/x86_64/multiarch/memcmp.c
> > > index 7757d1ec4e..bae29d8dad 100644
> > > --- a/sysdeps/x86_64/multiarch/memcmp.c
> > > +++ b/sysdeps/x86_64/multiarch/memcmp.c
> > > @@ -27,8 +27,6 @@
> > > # include "ifunc-memcmp.h"
> > >
> > > libc_ifunc_redirected (__redirect_memcmp, memcmp, IFUNC_SELECTOR ());
> > > -# undef bcmp
> > > -weak_alias (memcmp, bcmp)
> > >
> > > # ifdef SHARED
> > > __hidden_ver1 (memcmp, __GI_memcmp, __redirect_memcmp)
> > > diff --git a/sysdeps/x86_64/multiarch/memcmpeq.c b/sysdeps/x86_64/multiarch/memcmpeq.c
> > > index aa1c8ad4de..45cb6bad5b 100644
> > > --- a/sysdeps/x86_64/multiarch/memcmpeq.c
> > > +++ b/sysdeps/x86_64/multiarch/memcmpeq.c
> > > @@ -27,6 +27,8 @@
> > > # include "ifunc-memcmpeq.h"
> > >
> > > libc_ifunc_redirected (__redirect___memcmpeq, __memcmpeq, IFUNC_SELECTOR ());
> > > +# undef bcmp
> > > +weak_alias (__memcmpeq, bcmp)
> > >
> > > # ifdef SHARED
> > > __hidden_ver1 (__memcmpeq, __GI___memcmpeq, __redirect___memcmpeq)
> > > --
> > > 2.34.1
> > >
> >
> > Are we sure this is safe? One of the original rationales for adding
> > '__memcmpeq' as
> > a new interface instead of just optimizing 'bcmp' was that users may
> > be relying on
> > the fact that 'bcmp' has historically returned the LT/GT of the comparison.
>
> The bcmp man page has:
Si, but it may be a situation like 'memcpy' vs 'memmove'. Sure not supporting
overlap is legal by the spec but if enough users/applications rely on
the existing
functionality issues arise changing it.
Not inherently opposed to this patch (in fact in favor) but think some broader
community support given the previous reservations is needed.
>
> NAME
> bcmp - compare byte sequences
>
> SYNOPSIS
> #include <strings.h>
>
> int bcmp(const void *s1, const void *s2, size_t n);
>
> DESCRIPTION
> The bcmp() function compares the two byte sequences s1 and s2 of length
> n each. If they are equal, and in particular if n is zero, bcmp() re‐
> turns 0. Otherwise, it returns a nonzero result.
>
> RETURN VALUE
> The bcmp() function returns 0 if the byte sequences are equal, other‐
> wise a nonzero result is returned.
>
> > As well, if we are going to change 'bcmp' should we do it for all
> > architectures so that
> > its consistent?
>
> Only x86-64 has __memcmpeq which isn't an alias of memcmp.
Fair. Guess it can be added as optimized implementations come online.
>
> --
> H.J.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-06 20:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-06 19:26 [PATCH] x86-64: Make bcmp an alias of __memcmpeq H.J. Lu
2022-02-06 19:34 ` Noah Goldstein
2022-02-06 19:57 ` H.J. Lu
2022-02-06 20:16 ` Noah Goldstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).