[PATCH] m68k: remove arch specific non-optimized memcmp()

[PATCH] m68k: remove arch specific non-optimized memcmp()

[Greg Ungerer]

m68k: remove arch specific non-optimized memcmp()

The m68k arch implements its own memcmp() function. It is not optimized
in any way (it is the most strait forward coding of memcmp you can get).
Remove it and use the kernels standard memcmp() implementation.

This also goes part of the way to fixing a regression caused by commit
ea61bc461d09e8d331a307916530aaae808c72a2 ("m68k/m68knommu: merge MMU and
non-MMU string.h"), which breaks non-coldfire non-mmu builds (which is
the 68x328 and 68360 families). They currently have no memcmp() function
defined, since there is none in the m68knommu/lib functions.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/m68k/include/asm/string.h |    4 +---
 arch/m68k/lib/string.c         |   11 -----------
 2 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
index 65b1312..ffc3c3f 100644
--- a/arch/m68k/include/asm/string.h
+++ b/arch/m68k/include/asm/string.h
@@ -102,11 +102,9 @@ static inline int strcmp(const char *cs, const char *ct)
 
 #define __HAVE_ARCH_MEMMOVE
 extern void *memmove(void *, const void *, __kernel_size_t);
+#endif /* CONFIG_COLDFIRE */
 
-#define __HAVE_ARCH_MEMCMP
-extern int memcmp(const void *, const void *, __kernel_size_t);
 #define memcmp(d, s, n) __builtin_memcmp(d, s, n)
-#endif /* CONFIG_COLDFIRE */
 
 #define __HAVE_ARCH_MEMSET
 extern void *memset(void *, int, __kernel_size_t);
diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
index 4253f87..d399c5f 100644
--- a/arch/m68k/lib/string.c
+++ b/arch/m68k/lib/string.c
@@ -243,14 +243,3 @@ void *memmove(void *dest, const void *src, size_t n)
 	return xdest;
 }
 EXPORT_SYMBOL(memmove);
-
-int memcmp(const void *cs, const void *ct, size_t count)
-{
-	const unsigned char *su1, *su2;
-
-	for (su1 = cs, su2 = ct; count > 0; ++su1, ++su2, count--)
-		if (*su1 != *su2)
-			return *su1 < *su2 ? -1 : +1;
-	return 0;
-}
-EXPORT_SYMBOL(memcmp);

Re: [PATCH] m68k: remove arch specific non-optimized memcmp()

[Greg Ungerer]

On 07/02/11 10:57, Greg Ungerer wrote:
> m68k: remove arch specific non-optimized memcmp()
>
> The m68k arch implements its own memcmp() function. It is not optimized
> in any way (it is the most strait forward coding of memcmp you can get).
> Remove it and use the kernels standard memcmp() implementation.
>
> This also goes part of the way to fixing a regression caused by commit
> ea61bc461d09e8d331a307916530aaae808c72a2 ("m68k/m68knommu: merge MMU and
> non-MMU string.h"), which breaks non-coldfire non-mmu builds (which is
> the 68x328 and 68360 families). They currently have no memcmp() function
> defined, since there is none in the m68knommu/lib functions.
>
> Signed-off-by: Greg Ungerer<gerg@uclinux.org>

Does anyone have any comments on this?

I would like to push to Linus very soon now (so in the current
2.6.38-rc). It seems like a clean fix on its own, but in particular
it helps me fix the problems caused by commit 
ea61bc461d09e8d331a307916530aaae808c72a2
("m68k/m68knommu: merge MMU and non-MMU string.h"

I have compile and run tested on m68knommu targets, and compile
tested on m68k targets.

Regards
Greg


> ---
>   arch/m68k/include/asm/string.h |    4 +---
>   arch/m68k/lib/string.c         |   11 -----------
>   2 files changed, 1 insertions(+), 14 deletions(-)
>
> diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
> index 65b1312..ffc3c3f 100644
> --- a/arch/m68k/include/asm/string.h
> +++ b/arch/m68k/include/asm/string.h
> @@ -102,11 +102,9 @@ static inline int strcmp(const char *cs, const char *ct)
>
>   #define __HAVE_ARCH_MEMMOVE
>   extern void *memmove(void *, const void *, __kernel_size_t);
> +#endif /* CONFIG_COLDFIRE */
>
> -#define __HAVE_ARCH_MEMCMP
> -extern int memcmp(const void *, const void *, __kernel_size_t);
>   #define memcmp(d, s, n) __builtin_memcmp(d, s, n)
> -#endif /* CONFIG_COLDFIRE */
>
>   #define __HAVE_ARCH_MEMSET
>   extern void *memset(void *, int, __kernel_size_t);
> diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
> index 4253f87..d399c5f 100644
> --- a/arch/m68k/lib/string.c
> +++ b/arch/m68k/lib/string.c
> @@ -243,14 +243,3 @@ void *memmove(void *dest, const void *src, size_t n)
>   	return xdest;
>   }
>   EXPORT_SYMBOL(memmove);
> -
> -int memcmp(const void *cs, const void *ct, size_t count)
> -{
> -	const unsigned char *su1, *su2;
> -
> -	for (su1 = cs, su2 = ct; count>  0; ++su1, ++su2, count--)
> -		if (*su1 != *su2)
> -			return *su1<  *su2 ? -1 : +1;
> -	return 0;
> -}
> -EXPORT_SYMBOL(memcmp);
>


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH] m68k: remove arch specific non-optimized memcmp()

[Geert Uytterhoeven]

On Fri, Feb 11, 2011 at 02:24, Greg Ungerer <gerg@snapgear.com> wrote:
> On 07/02/11 10:57, Greg Ungerer wrote:
>>
>> m68k: remove arch specific non-optimized memcmp()
>>
>> The m68k arch implements its own memcmp() function. It is not optimized
>> in any way (it is the most strait forward coding of memcmp you can get).
>> Remove it and use the kernels standard memcmp() implementation.
>>
>> This also goes part of the way to fixing a regression caused by commit
>> ea61bc461d09e8d331a307916530aaae808c72a2 ("m68k/m68knommu: merge MMU and
>> non-MMU string.h"), which breaks non-coldfire non-mmu builds (which is
>> the 68x328 and 68360 families). They currently have no memcmp() function
>> defined, since there is none in the m68knommu/lib functions.
>>
>> Signed-off-by: Greg Ungerer<gerg@uclinux.org>
>
> Does anyone have any comments on this?

Sorry, haven't tried it yet.

Shouldn't you also remove the "#define memcmp(d, s, n)"?

> I would like to push to Linus very soon now (so in the current
> 2.6.38-rc). It seems like a clean fix on its own, but in particular
> it helps me fix the problems caused by commit
> ea61bc461d09e8d331a307916530aaae808c72a2
> ("m68k/m68knommu: merge MMU and non-MMU string.h"
>
> I have compile and run tested on m68knommu targets, and compile
> tested on m68k targets.

If it compiles, it's good; if it boots, it's perfect ;-)

>> ---
>>  arch/m68k/include/asm/string.h |    4 +---
>>  arch/m68k/lib/string.c         |   11 -----------
>>  2 files changed, 1 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/m68k/include/asm/string.h
>> b/arch/m68k/include/asm/string.h
>> index 65b1312..ffc3c3f 100644
>> --- a/arch/m68k/include/asm/string.h
>> +++ b/arch/m68k/include/asm/string.h
>> @@ -102,11 +102,9 @@ static inline int strcmp(const char *cs, const char
>> *ct)
>>
>>  #define __HAVE_ARCH_MEMMOVE
>>  extern void *memmove(void *, const void *, __kernel_size_t);
>> +#endif /* CONFIG_COLDFIRE */
>>
>> -#define __HAVE_ARCH_MEMCMP
>> -extern int memcmp(const void *, const void *, __kernel_size_t);
>>  #define memcmp(d, s, n) __builtin_memcmp(d, s, n)
>> -#endif /* CONFIG_COLDFIRE */
>>
>>  #define __HAVE_ARCH_MEMSET
>>  extern void *memset(void *, int, __kernel_size_t);
>> diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
>> index 4253f87..d399c5f 100644
>> --- a/arch/m68k/lib/string.c
>> +++ b/arch/m68k/lib/string.c
>> @@ -243,14 +243,3 @@ void *memmove(void *dest, const void *src, size_t n)
>>        return xdest;
>>  }
>>  EXPORT_SYMBOL(memmove);
>> -
>> -int memcmp(const void *cs, const void *ct, size_t count)
>> -{
>> -       const unsigned char *su1, *su2;
>> -
>> -       for (su1 = cs, su2 = ct; count>  0; ++su1, ++su2, count--)
>> -               if (*su1 != *su2)
>> -                       return *su1<  *su2 ? -1 : +1;
>> -       return 0;
>> -}
>> -EXPORT_SYMBOL(memcmp);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k: remove arch specific non-optimized memcmp()

[Greg Ungerer]

Hi Geert,

> On Fri, Feb 11, 2011 at 02:24, Greg Ungerer <gerg@snapgear.com> wrote:
>> On 07/02/11 10:57, Greg Ungerer wrote:
>>>
>>> m68k: remove arch specific non-optimized memcmp()
>>>
>>> The m68k arch implements its own memcmp() function. It is not optimized
>>> in any way (it is the most strait forward coding of memcmp you can get).
>>> Remove it and use the kernels standard memcmp() implementation.
>>>
>>> This also goes part of the way to fixing a regression caused by commit
>>> ea61bc461d09e8d331a307916530aaae808c72a2 ("m68k/m68knommu: merge MMU and
>>> non-MMU string.h"), which breaks non-coldfire non-mmu builds (which is
>>> the 68x328 and 68360 families). They currently have no memcmp() function
>>> defined, since there is none in the m68knommu/lib functions.
>>>
>>> Signed-off-by: Greg Ungerer<gerg@uclinux.org>
>>
>> Does anyone have any comments on this?
>
> Sorry, haven't tried it yet.
>
> Shouldn't you also remove the "#define memcmp(d, s, n)"?

I considered doing that. But leaving that in and going through
__builtin_memcmp() gives the compiler a chance to optimize
some simple calls to memcmp to inline instructions. In practice
it might not matter much, and I don't mind at all leaving it out
if you prefer.


>> I would like to push to Linus very soon now (so in the current
>> 2.6.38-rc). It seems like a clean fix on its own, but in particular
>> it helps me fix the problems caused by commit
>> ea61bc461d09e8d331a307916530aaae808c72a2
>> ("m68k/m68knommu: merge MMU and non-MMU string.h"
>>
>> I have compile and run tested on m68knommu targets, and compile
>> tested on m68k targets.
>
> If it compiles, it's good; if it boots, it's perfect ;-)

Unfortunately I don't have any true 68k machines to run
this on. I do have a lot of ColdFire boards though :-)

Regards
Greg



>>> ---
>>> Â arch/m68k/include/asm/string.h | Â  Â 4 +---
>>>  arch/m68k/lib/string.c         |   11 -----------
>>> Â 2 files changed, 1 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/arch/m68k/include/asm/string.h
>>> b/arch/m68k/include/asm/string.h
>>> index 65b1312..ffc3c3f 100644
>>> --- a/arch/m68k/include/asm/string.h
>>> +++ b/arch/m68k/include/asm/string.h
>>> @@ -102,11 +102,9 @@ static inline int strcmp(const char *cs, const char
>>> *ct)
>>>
>>> Â #define __HAVE_ARCH_MEMMOVE
>>> Â extern void *memmove(void *, const void *, __kernel_size_t);
>>> +#endif /* CONFIG_COLDFIRE */
>>>
>>> -#define __HAVE_ARCH_MEMCMP
>>> -extern int memcmp(const void *, const void *, __kernel_size_t);
>>> Â #define memcmp(d, s, n) __builtin_memcmp(d, s, n)
>>> -#endif /* CONFIG_COLDFIRE */
>>>
>>> Â #define __HAVE_ARCH_MEMSET
>>> Â extern void *memset(void *, int, __kernel_size_t);
>>> diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
>>> index 4253f87..d399c5f 100644
>>> --- a/arch/m68k/lib/string.c
>>> +++ b/arch/m68k/lib/string.c
>>> @@ -243,14 +243,3 @@ void *memmove(void *dest, const void *src, size_t n)
>>> Â  Â  Â  Â return xdest;
>>> Â }
>>> Â EXPORT_SYMBOL(memmove);
>>> -
>>> -int memcmp(const void *cs, const void *ct, size_t count)
>>> -{
>>> - Â  Â  Â  const unsigned char *su1, *su2;
>>> -
>>> - Â  Â  Â  for (su1 = cs, su2 = ct; count> Â 0; ++su1, ++su2, count--)
>>> - Â  Â  Â  Â  Â  Â  Â  if (*su1 != *su2)
>>> - Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  return *su1< Â *su2 ? -1 : +1;
>>> - Â  Â  Â  return 0;
>>> -}
>>> -EXPORT_SYMBOL(memcmp);
>
> Gr{oetje,eeting}s,
>
> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â Â  Â Â  -- Linus Torvalds

------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH] m68k: remove arch specific non-optimized memcmp()

[Geert Uytterhoeven]

On Mon, Feb 7, 2011 at 01:57, Greg Ungerer <gerg@snapgear.com> wrote:
> m68k: remove arch specific non-optimized memcmp()
>
> The m68k arch implements its own memcmp() function. It is not optimized
> in any way (it is the most strait forward coding of memcmp you can get).
> Remove it and use the kernels standard memcmp() implementation.
>
> This also goes part of the way to fixing a regression caused by commit
> ea61bc461d09e8d331a307916530aaae808c72a2 ("m68k/m68knommu: merge MMU and
> non-MMU string.h"), which breaks non-coldfire non-mmu builds (which is
> the 68x328 and 68360 families). They currently have no memcmp() function
> defined, since there is none in the m68knommu/lib functions.
>
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>

Go ahead!

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

> ---
>  arch/m68k/include/asm/string.h |    4 +---
>  arch/m68k/lib/string.c         |   11 -----------
>  2 files changed, 1 insertions(+), 14 deletions(-)
>
> diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
> index 65b1312..ffc3c3f 100644
> --- a/arch/m68k/include/asm/string.h
> +++ b/arch/m68k/include/asm/string.h
> @@ -102,11 +102,9 @@ static inline int strcmp(const char *cs, const char *ct)
>
>  #define __HAVE_ARCH_MEMMOVE
>  extern void *memmove(void *, const void *, __kernel_size_t);
> +#endif /* CONFIG_COLDFIRE */
>
> -#define __HAVE_ARCH_MEMCMP
> -extern int memcmp(const void *, const void *, __kernel_size_t);
>  #define memcmp(d, s, n) __builtin_memcmp(d, s, n)
> -#endif /* CONFIG_COLDFIRE */
>
>  #define __HAVE_ARCH_MEMSET
>  extern void *memset(void *, int, __kernel_size_t);
> diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
> index 4253f87..d399c5f 100644
> --- a/arch/m68k/lib/string.c
> +++ b/arch/m68k/lib/string.c
> @@ -243,14 +243,3 @@ void *memmove(void *dest, const void *src, size_t n)
>        return xdest;
>  }
>  EXPORT_SYMBOL(memmove);
> -
> -int memcmp(const void *cs, const void *ct, size_t count)
> -{
> -       const unsigned char *su1, *su2;
> -
> -       for (su1 = cs, su2 = ct; count > 0; ++su1, ++su2, count--)
> -               if (*su1 != *su2)
> -                       return *su1 < *su2 ? -1 : +1;
> -       return 0;
> -}
> -EXPORT_SYMBOL(memcmp);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] m68k: remove arch specific non-optimized memcmp()

[Greg Ungerer]

On 16/02/11 05:50, Geert Uytterhoeven wrote:
> On Mon, Feb 7, 2011 at 01:57, Greg Ungerer<gerg@snapgear.com>  wrote:
>> m68k: remove arch specific non-optimized memcmp()
>>
>> The m68k arch implements its own memcmp() function. It is not optimized
>> in any way (it is the most strait forward coding of memcmp you can get).
>> Remove it and use the kernels standard memcmp() implementation.
>>
>> This also goes part of the way to fixing a regression caused by commit
>> ea61bc461d09e8d331a307916530aaae808c72a2 ("m68k/m68knommu: merge MMU and
>> non-MMU string.h"), which breaks non-coldfire non-mmu builds (which is
>> the 68x328 and 68360 families). They currently have no memcmp() function
>> defined, since there is none in the m68knommu/lib functions.
>>
>> Signed-off-by: Greg Ungerer<gerg@uclinux.org>
>
> Go ahead!
>
> Acked-by: Geert Uytterhoeven<geert@linux-m68k.org>

Thanks Geert!

I'll get this off to Linus today.

Regards
Greg


>> ---
>>   arch/m68k/include/asm/string.h |    4 +---
>>   arch/m68k/lib/string.c         |   11 -----------
>>   2 files changed, 1 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
>> index 65b1312..ffc3c3f 100644
>> --- a/arch/m68k/include/asm/string.h
>> +++ b/arch/m68k/include/asm/string.h
>> @@ -102,11 +102,9 @@ static inline int strcmp(const char *cs, const char *ct)
>>
>>   #define __HAVE_ARCH_MEMMOVE
>>   extern void *memmove(void *, const void *, __kernel_size_t);
>> +#endif /* CONFIG_COLDFIRE */
>>
>> -#define __HAVE_ARCH_MEMCMP
>> -extern int memcmp(const void *, const void *, __kernel_size_t);
>>   #define memcmp(d, s, n) __builtin_memcmp(d, s, n)
>> -#endif /* CONFIG_COLDFIRE */
>>
>>   #define __HAVE_ARCH_MEMSET
>>   extern void *memset(void *, int, __kernel_size_t);
>> diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
>> index 4253f87..d399c5f 100644
>> --- a/arch/m68k/lib/string.c
>> +++ b/arch/m68k/lib/string.c
>> @@ -243,14 +243,3 @@ void *memmove(void *dest, const void *src, size_t n)
>>         return xdest;
>>   }
>>   EXPORT_SYMBOL(memmove);
>> -
>> -int memcmp(const void *cs, const void *ct, size_t count)
>> -{
>> -       const unsigned char *su1, *su2;
>> -
>> -       for (su1 = cs, su2 = ct; count>  0; ++su1, ++su2, count--)
>> -               if (*su1 != *su2)
>> -                       return *su1<  *su2 ? -1 : +1;
>> -       return 0;
>> -}
>> -EXPORT_SYMBOL(memcmp);
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
>


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com