[U-Boot] [PATCH] lib/string: memmove: use memcpy if it is safe to do so

[U-Boot] [PATCH] lib/string: memmove: use memcpy if it is safe to do so

[Peter Korsgaard]

memmove is used in a number of performance critical places, like copying the
linux kernel from nor flash to ram, so optimizing it can be interesting.

Unfortunately, an arch specific version of memmove often isn't used, and
not supported at all on a number of archs (arm/mips/nds32/nios2/x86) -
But memcpy typically is.

Often memmove is called for copies where src/dest don't overlap, so we
could use the more efficient memcpy instead. Detect these situations and
forward those request to memcpy instead.

Adds 40 bytes to memmove and speeds up boot with ~300ms on a 400MHz arm9.

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
---
 lib/string.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index 2c4f0ec..239cc11 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -504,12 +504,16 @@ void * memmove(void * dest,const void *src,size_t count)
 		return dest;
 
 	if (dest <= src) {
+		if (dest + count <= src)
+			return memcpy(dest, src, count);
 		tmp = (char *) dest;
 		s = (char *) src;
 		while (count--)
 			*tmp++ = *s++;
 		}
 	else {
+		if (src + count <= dest)
+			return memcpy(dest, src, count);
 		tmp = (char *) dest + count;
 		s = (char *) src + count;
 		while (count--)
-- 
1.7.6.3

[U-Boot] [PATCH] lib/string: memmove: use memcpy if it is safe to do so

[Wolfgang Denk]

Dear Peter Korsgaard,

In message <1319789046-17715-1-git-send-email-jacmet@sunsite.dk> you wrote:
> memmove is used in a number of performance critical places, like copying the
> linux kernel from nor flash to ram, so optimizing it can be interesting.
> 
> Unfortunately, an arch specific version of memmove often isn't used, and
> not supported at all on a number of archs (arm/mips/nds32/nios2/x86) -
> But memcpy typically is.

So you are adding code, making the system even less efficient?  This
sounds to be acounter-productive approach.

Can you not instead arrange for arch specific, optimized versions
memmove() to be used?

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
Uncertain fortune is thoroughly mastered by the equity of the  calcu-
lation.                                               - Blaise Pascal

[U-Boot] [PATCH] lib/string: memmove: use memcpy if it is safe to do so

[Peter Korsgaard]

>>>>> "Wolfgang" == Wolfgang Denk <wd@denx.de> writes:

Hi,

 >> memmove is used in a number of performance critical places, like copying the
 >> linux kernel from nor flash to ram, so optimizing it can be interesting.
 >> 
 >> Unfortunately, an arch specific version of memmove often isn't used, and
 >> not supported at all on a number of archs (arm/mips/nds32/nios2/x86) -
 >> But memcpy typically is.

 Wolfgang> So you are adding code, making the system even less efficient?  This
 Wolfgang> sounds to be acounter-productive approach.

Well, slightly less efficient for the (uncommon) situation where
src/dest overlaps in return for significantly more efficiency for the
situation where they don't.

 Wolfgang> Can you not instead arrange for arch specific, optimized versions
 Wolfgang> memmove() to be used?

That's another option, but more work for implementers - So I would
prefer this trivial patch instead.

But I'll drop it if you disagree.

-- 
Bye, Peter Korsgaard

[U-Boot] [PATCH] lib/string: memmove: use memcpy if it is safe to do so

[Mike Frysinger]

On Fri, Oct 28, 2011 at 22:25, Wolfgang Denk wrote:
> Peter Korsgaard wrote:
>> memmove is used in a number of performance critical places, like copying the
>> linux kernel from nor flash to ram, so optimizing it can be interesting.
>>
>> Unfortunately, an arch specific version of memmove often isn't used, and
>> not supported at all on a number of archs (arm/mips/nds32/nios2/x86) -
>> But memcpy typically is.
>
> So you are adding code, making the system even less efficient? ?This
> sounds to be acounter-productive approach.
>
> Can you not instead arrange for arch specific, optimized versions
> memmove() to be used?

i think this is kind of a loose-loose situation.  in common code, we
have to use memmove() if the two regions could *possibly* overlap.  so
in practice, we end up calling memmove() in more cases than strictly
necessary.  but the only real choice is to update all the call sites
with region overlap checks.

at least with Peter's patch, we centralize all these checks in one
place producing smaller code.  in some cases, we probably add a little
bit of runtime overhead when the new if() statement returns false, but
it seems to be that one successful call quickly outweighs multiple
unsuccessful if() checks.

i also think this falls into the same category as the previous commit
b038db852b5b7120f1ff825d8e2a5c2cd14c2f0f:
memcpy/memmove: Do not copy to same address

so the ideal probably would be to have an optimized memmove in place,
but for the (many) arches which don't have this, we seem to get a
"win" overall ?
-mike

[U-Boot] [PATCH] lib/string: memmove: use memcpy if it is safe to do so

[Wolfgang Denk]

Dear Peter Korsgaard,

In message <87vcr8ofw9.fsf@macbook.be.48ers.dk> you wrote:
>
>  Wolfgang> So you are adding code, making the system even less efficient?  This
>  Wolfgang> sounds to be acounter-productive approach.
> 
> Well, slightly less efficient for the (uncommon) situation where
> src/dest overlaps in return for significantly more efficiency for the
> situation where they don't.

The test which situation we have needs to be always performed, and
adds overhead, especially for short copies.

>  Wolfgang> Can you not instead arrange for arch specific, optimized versions
>  Wolfgang> memmove() to be used?
> 
> That's another option, but more work for implementers - So I would
> prefer this trivial patch instead.
> 
> But I'll drop it if you disagree.

I prefer to keep this code as is, and add optimized versions of
memmove() where needed / wanted.

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
Anyone who isn't confused here doesn't really know what's going on.