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

[Greg Ungerer <gerg@snapgear.com>]

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