2008-11-10 06:12:36

by Harvey Harrison

[permalink] [raw]
Subject: [RFC-PATCH] x86: really use __builtin_memcmp on x86_32

Impact: prevent generic code from overriding __builtin_memcmp

lib/string.c was using a generic implementation of memcmp
because __HAVE_ARCH_MEMCMP was not defined and it was then doing
#undef memcmp and defining a generic version.

Signed-off-by: Harvey Harrison <[email protected]>
---
arch/x86/include/asm/string_32.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
index c86f452..b671baf 100644
--- a/arch/x86/include/asm/string_32.h
+++ b/arch/x86/include/asm/string_32.h
@@ -195,6 +195,7 @@ static inline void *__memcpy3d(void *to, const void *from, size_t len)
#define __HAVE_ARCH_MEMMOVE
void *memmove(void *dest, const void *src, size_t n);

+#define __HAVE_ARCH_MEMCMP
#define memcmp __builtin_memcmp

#define __HAVE_ARCH_MEMCHR
--
1.6.0.3.866.gc189b



2008-11-10 07:45:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC-PATCH] x86: really use __builtin_memcmp on x86_32


* Harvey Harrison <[email protected]> wrote:

> Impact: prevent generic code from overriding __builtin_memcmp
>
> lib/string.c was using a generic implementation of memcmp
> because __HAVE_ARCH_MEMCMP was not defined and it was then doing
> #undef memcmp and defining a generic version.
>
> Signed-off-by: Harvey Harrison <[email protected]>
> ---
> arch/x86/include/asm/string_32.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)

> +#define __HAVE_ARCH_MEMCMP
> #define memcmp __builtin_memcmp
>
> #define __HAVE_ARCH_MEMCHR

on a quick look, since x86 sets __HAVE_ARCH_STRSTR, nothing in
lib/string.c would actually use this generic (and slower) memcmp
implementation, correct? So there should be no impact to object code,
it's a cleanup - right?

Ingo

2008-11-10 08:05:54

by Ingo Molnar

[permalink] [raw]
Subject: Re: [RFC-PATCH] x86: really use __builtin_memcmp on x86_32


* Ingo Molnar <[email protected]> wrote:

> * Harvey Harrison <[email protected]> wrote:
>
> > Impact: prevent generic code from overriding __builtin_memcmp
> >
> > lib/string.c was using a generic implementation of memcmp
> > because __HAVE_ARCH_MEMCMP was not defined and it was then doing
> > #undef memcmp and defining a generic version.
> >
> > Signed-off-by: Harvey Harrison <[email protected]>
> > ---
> > arch/x86/include/asm/string_32.h | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
>
> > +#define __HAVE_ARCH_MEMCMP
> > #define memcmp __builtin_memcmp
> >
> > #define __HAVE_ARCH_MEMCHR

doesnt work that well:

arch/x86/kernel/built-in.o: In function `efi_guidcmp':
efi.c:(.text+0x108b2): undefined reference to `memcmp'
arch/x86/kernel/built-in.o: In function `smp_check_mpc':
mpparse.c:(.init.text+0x4ec6): undefined reference to `memcmp'
arch/x86/kernel/built-in.o: In function `powernowk8_cpu_init':

with the attached config.

Ingo


Attachments:
(No filename) (993.00 B)
config (59.15 kB)
Download all attachments

2008-11-10 08:21:38

by Andi Kleen

[permalink] [raw]
Subject: Re: [RFC-PATCH] x86: really use __builtin_memcmp on x86_32

Harvey Harrison <[email protected]> writes:

> Impact: prevent generic code from overriding __builtin_memcmp
>
> lib/string.c was using a generic implementation of memcmp
> because __HAVE_ARCH_MEMCMP was not defined and it was then doing
> #undef memcmp and defining a generic version.

gcc sometimes decides to not inline memcmp, so an out of line
version is always needed even when __builtin_memcmp is used.

While in theory x86 could supply an optimized version,
memcmp is relatively rarely used (and in many cases inlined),
so adding an assembler version for x86 is likely not worth
it.

-Andi

--
[email protected]

2008-11-10 16:53:29

by Harvey Harrison

[permalink] [raw]
Subject: Re: [RFC-PATCH] x86: really use __builtin_memcmp on x86_32

On Mon, 2008-11-10 at 09:05 +0100, Ingo Molnar wrote:
> * Ingo Molnar <[email protected]> wrote:
>
> > * Harvey Harrison <[email protected]> wrote:
> >
> > > Impact: prevent generic code from overriding __builtin_memcmp
> > >
> > > lib/string.c was using a generic implementation of memcmp
> > > because __HAVE_ARCH_MEMCMP was not defined and it was then doing
> > > #undef memcmp and defining a generic version.
> > >
> > > Signed-off-by: Harvey Harrison <[email protected]>
> > > ---
> > > arch/x86/include/asm/string_32.h | 1 +
> > > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > > +#define __HAVE_ARCH_MEMCMP
> > > #define memcmp __builtin_memcmp
> > >
> > > #define __HAVE_ARCH_MEMCHR
>
> doesnt work that well:
>
> arch/x86/kernel/built-in.o: In function `efi_guidcmp':
> efi.c:(.text+0x108b2): undefined reference to `memcmp'
> arch/x86/kernel/built-in.o: In function `smp_check_mpc':
> mpparse.c:(.init.text+0x4ec6): undefined reference to `memcmp'
> arch/x86/kernel/built-in.o: In function `powernowk8_cpu_init':
>
> with the attached config.
>

Just let this one go to the bitbucket.

Harvey