[U-Boot] [PATCH 2/3] memset: use 32-bit copies if possible

[Wolfgang Denk]

Dear Alessandro Rubini,

In message <d10837d7edf963ce9b0e87773c9771e451f9f9ac.1254904388.git.rubini@ unipv.it> you wrote:
> From: Alessandro Rubini <rubini at unipv.it>
> 
> Signed-off-by: Alessandro Rubini <rubini at unipv.it>
> Acked-by: Andrea Gallo <andrea.gallo at stericsson.com>

Please fix the Subject: memset() does not copy data.

>  lib_generic/string.c |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/lib_generic/string.c b/lib_generic/string.c
> index fdccab6..68e0255 100644
> --- a/lib_generic/string.c
> +++ b/lib_generic/string.c
> @@ -404,7 +404,21 @@ char *strswab(const char *s)
>  void * memset(void * s,int c,size_t count)
>  {
>  	char *xs = (char *) s;
> +	u32 *s32 = (u32 *) s;
> +	int c32 = 0; /* most common case */
>  
> +	/* do it 32 bits at a time if possible */
> +	if ( ((count & 3) | ((int)s & 3)) == 0) {
> +		count /= 4;
> +		if (c) { /* not 0: build 32-bit value */
> +			c32 = c | (c<<8);
> +			c32 |= c32 << 16;

I reject this code, as it changes behaviour. You may argument that
it's only for special cases, but anyway:

Fill pattern	for standard memset()	for your code:
c = -2		0xFEFEFEFE		0xFFFFFFFE
c = -3		0xFDFDFDFD		0xFFFFFFFD
...
c = -2147483648	0x00000000		0x80000000

> +		}

I suggest to omit the special case for zero and handle it just the
same. This results in smaller code size and is easier to read.

> +		while (count--)
> +			*s32++ = c32;
> +		return s;
> +	}
> +	/* else, fill 8 bits at a time */

Blank line here, please.

>  	while (count--)
>  		*xs++ = c;

Please fix and resubmit.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
God made machine language; all the rest is the work of man.