[U-Boot-Users] [PATCH] mips: Bring over optimized memset() routine from Linux.

Shinya Kuribayashi shinya.kuribayashi at necel.com
Tue Jun 10 12:51:09 CEST 2008


Hi Jason,

Jason McMullan wrote:
> This commit pulls over the memset() MIPS routine from Linux 2.6.26,
> which provides a 10x to 20x speedup over the generic byte-at-a-time
> routine. This is especially useful on platforms with manual ECC
> scrubbing, that require all of memory to be written at least once
> after a power cycle.
> ---
>  include/asm-mips/string.h |    2 +-
>  lib_mips/Makefile         |    2 +-
>  lib_mips/memset.S         |  174 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 176 insertions(+), 2 deletions(-)
>  create mode 100644 lib_mips/memset.S

IIRC, Linux's memset relies on AdEL/AdES exceptions. We have Status.EXL
enabled, but don't have proper exception handlers, yet. So my question
is does this code always works expectedly, or works with some alignment
restriction?

And some nitpickings. See below.

> diff --git a/lib_mips/memset.S b/lib_mips/memset.S
> new file mode 100644
> index 0000000..f1c07d7
> --- /dev/null
> +++ b/lib_mips/memset.S
> @@ -0,0 +1,174 @@
> +/*
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 1998, 1999, 2000 by Ralf Baechle
> + * Copyright (C) 1999, 2000 Silicon Graphics, Inc.
> + * Copyright (C) 2007  Maciej W. Rozycki
> + */
> +#include <asm/asm.h>
> +//#include <asm/asm-offsets.h>

Please remove unused #include. Even '#if 0'-ing is not allowed in
U-Boot policy.

> +#include <asm/regdef.h>
> +
> +#if LONGSIZE == 4
> +#define LONG_S_L swl
> +#define LONG_S_R swr
> +#else
> +#define LONG_S_L sdl
> +#define LONG_S_R sdr
> +#endif
> +
> +#define EX(insn,reg,addr,handler)			\
> +9:	insn	reg, addr;				\
> +	.section __ex_table,"a"; 			\
> +	PTR	9b, handler; 				\
> +	.previous
> +
> +	.macro	f_fill64 dst, offset, val, fixup
> +	EX(LONG_S, \val, (\offset +  0 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  1 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  2 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  3 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  4 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  5 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  6 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  7 * LONGSIZE)(\dst), \fixup)
> +#if LONGSIZE == 4
> +	EX(LONG_S, \val, (\offset +  8 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset +  9 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset + 10 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset + 11 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset + 12 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset + 13 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset + 14 * LONGSIZE)(\dst), \fixup)
> +	EX(LONG_S, \val, (\offset + 15 * LONGSIZE)(\dst), \fixup)
> +#endif
> +	.endm
> +
> +/*
> + * memset(void *s, int c, size_t n)
> + *
> + * a0: start of area to clear
> + * a1: char to fill with
> + * a2: size of area to clear
> + */
> +	.set	noreorder
> +	.align	5
> +LEAF(memset)
> +	beqz		a1, 1f
> +	 move		v0, a0			/* result */

        ^

> +	andi		a1, 0xff		/* spread fillword */
> +	LONG_SLL		t1, a1, 8
> +	or		a1, t1
> +	LONG_SLL		t1, a1, 16
> +#if LONGSIZE == 8
> +	or		a1, t1
> +	LONG_SLL		t1, a1, 32
> +#endif
> +	or		a1, t1
> +1:
> +
> +FEXPORT(__bzero)
> +	sltiu		t0, a2, LONGSIZE	/* very small region? */
> +	bnez		t0, .Lsmall_memset
> +	 andi		t0, a0, LONGMASK	/* aligned? */

        ^

[further part snipped]

Please fix wrong indentations with proper tabs. I know this is exactly
the same as Linux's memset, but we prefer to fix it correctly in U-Boot.

[ I used to do like you did, but changed my mind. Now I think this is
  better practice. Incoherent indentations with Linux is not a big deal
  IMO. Just diff -w option blows them away. ]

Thanks in advance,

-- 
Shinya Kuribayashi
NEC Electronics




More information about the U-Boot mailing list