Grant Grundler wrote:
On Thu, Jan 03, 2008 at 12:37:12PM +0100, rubisher wrote:Hello *, I am not sure to have perfectly understand all subtle details of likely() & unlikely() macros but I think there's some brace at the bad place in following chunk: --- arch/parisc/lib/memcpy.c.Orig 2007-10-18 15:27:30.000000000 +0000 +++ arch/parisc/lib/memcpy.c 2008-01-03 10:17:52.000000000 +0000 @@ -299,7 +299,7 @@/* Check alignment */t1 = (src ^ dst); - if (unlikely(t1 & (sizeof(double)-1))) + if (unlikely(t1 & (sizeof(double) - 1)))Please submit separate patches for the white space clean up.goto unaligned_copy;/* src and dst have same alignment. */@@ -405,7 +405,7 @@unaligned_copy:/* possibly we are aligned on a word, but not on a double... */ - if (likely(t1 & (sizeof(unsigned int)-1)) == 0) { + if (likely((t1 & (sizeof(unsigned int) - 1)) == 0)) { t2 = src & (sizeof(unsigned int) - 1);if (unlikely(t2 != 0)) {=== <> === First hunk is just to add some whitespace? Otoh for the second hunk, my reading of the original stuff was that:likely(t1 & (sizeof(unsigned int)-1))i.e. likely's macro embraced only "t1 & (sizeof(unsigned int)-1)" in place of "(t1 & (sizeof(unsigned int)-1)) == 0". What's your opinion?I think you are right. normally likely() and unlikely() are intended to be used with boolean expressions and that's obviously not the case. Please resubmit this change separately and add "signed-off-by" lines so kyle can include this in the next round of parisc patches. cheers, grant
Grant, Ok here there are: a white space cleanup/beautify patch: Signed-off-by: Joel Soete <soete dot joel at scarlet dot be> Index: linux-current/arch/parisc/lib/memcpy.c =================================================================== --- linux-current.orig/arch/parisc/lib/memcpy.c 2008-01-05 13:37:04.000000000 +0000 +++ linux-current/arch/parisc/lib/memcpy.c 2008-01-05 13:55:21.000000000 +0000 @@ -21,28 +21,28 @@ * Copyright (C) 1991, 1997, 2003 Free Software Foundation, Inc. * * Several strategies are tried to try to get the best performance for various - * conditions. In the optimal case, we copy 64-bytes in an unrolled loop using + * conditions. In the optimal case, we copy 64-bytes in an unrolled loop using * fp regs. This is followed by loops that copy 32- or 16-bytes at a time using - * general registers. Unaligned copies are handled either by aligning the - * destination and then using shift-and-write method, or in a few cases by + * general registers. Unaligned copies are handled either by aligning the + * destination and then using shift-and-write method, or in a few cases by * falling back to a byte-at-a-time copy. * * I chose to implement this in C because it is easier to maintain and debug, * and in my experiments it appears that the C code generated by gcc (3.3/3.4 - * at the time of writing) is fairly optimal. Unfortunately some of the + * at the time of writing) is fairly optimal. Unfortunately some of the * semantics of the copy routine (exception handling) is difficult to express * in C, so we have to play some tricks to get it to work. * * All the loads and stores are done via explicit asm() code in order to use - * the right space registers. - * - * Testing with various alignments and buffer sizes shows that this code is + * the right space registers. + * + * Testing with various alignments and buffer sizes shows that this code is * often >10x faster than a simple byte-at-a-time copy, even for strangely * aligned operands. It is interesting to note that the glibc version - * of memcpy (written in C) is actually quite fast already. This routine is - * able to beat it by 30-40% for aligned copies because of the loop unrolling, - * but in some cases the glibc version is still slightly faster. This lends - * more credibility that gcc can generate very good code as long as we are + * of memcpy (written in C) is actually quite fast already. This routine is + * able to beat it by 30-40% for aligned copies because of the loop unrolling, + * but in some cases the glibc version is still slightly faster. This lends + * more credibility that gcc can generate very good code as long as we are * careful. * * TODO: @@ -154,7 +154,7 @@ #endif /* Copy from a not-aligned src to an aligned dst, using shifts. Handles 4 words - * per loop. This code is derived from glibc. + * per loop. This code is derived from glibc. */static inline unsigned long copy_dstaligned(unsigned long dst, unsigned long src, unsigned long len, unsigned long o_dst, unsigned long o_src, unsigned long o_len)
{
@@ -299,7 +299,7 @@
/* Check alignment */
t1 = (src ^ dst);
- if (unlikely(t1 & (sizeof(double)-1)))
+ if (unlikely(t1 & (sizeof(double) - 1)))
goto unaligned_copy;
/* src and dst have same alignment. */
@@ -322,7 +322,7 @@
#if 0
/* Copy 8 doubles at a time */
- while (len >= 8*sizeof(double)) {
+ while (len >= 8 * sizeof(double)) {
register double r1, r2, r3, r4, r5, r6, r7, r8;
/* prefetch_src((char *)pds + L1_CACHE_BYTES); */
flddma(s_space, pds, r1, pmc_load_exc);
@@ -346,7 +346,7 @@
fstdma(d_space, r6, pdd, pmc_store_exc);
fstdma(d_space, r7, pdd, pmc_store_exc);
fstdma(d_space, r8, pdd, pmc_store_exc);
- len -= 8*sizeof(double);
+ len -= 8 * sizeof(double);
}
#endif
@@ -354,7 +354,7 @@
pwd = (unsigned int *)pdd;
word_copy:
- while (len >= 8*sizeof(unsigned int)) {
+ while (len >= 8 * sizeof(unsigned int)) {
register unsigned int r1,r2,r3,r4,r5,r6,r7,r8;
/* prefetch_src((char *)pws + L1_CACHE_BYTES); */
ldwma(s_space, pws, r1, pmc_load_exc);
@@ -374,7 +374,7 @@
stwma(d_space, r6, pwd, pmc_store_exc);
stwma(d_space, r7, pwd, pmc_store_exc);
stwma(d_space, r8, pwd, pmc_store_exc);
- len -= 8*sizeof(unsigned int);
+ len -= 8 * sizeof(unsigned int);
}
while (len >= 4*sizeof(unsigned int)) {
@@ -387,7 +387,7 @@
stwma(d_space, r2, pwd, pmc_store_exc);
stwma(d_space, r3, pwd, pmc_store_exc);
stwma(d_space, r4, pwd, pmc_store_exc);
- len -= 4*sizeof(unsigned int);
+ len -= 4 * sizeof(unsigned int);
}
pcs = (unsigned char *)pws;
@@ -438,7 +438,7 @@
src = (unsigned long)pcs;
}
- ret = copy_dstaligned(dst, src, len / sizeof(unsigned int),
+ ret = copy_dstaligned(dst, src, len / sizeof(unsigned int),
o_dst, o_src, o_len);
if (ret)
return ret;
=== <> ===
and
a fixe of likely's macro usage:
Signed-off-by: Joel Soete <soete dot joel at scarlet dot be>
Index: linux-current/arch/parisc/lib/memcpy.c
===================================================================
--- linux-current.orig/arch/parisc/lib/memcpy.c 2008-01-05 13:57:26.000000000 +0000
+++ linux-current/arch/parisc/lib/memcpy.c 2008-01-05 13:58:01.000000000 +0000
@@ -405,7 +405,7 @@
unaligned_copy:
/* possibly we are aligned on a word, but not on a double... */
- if (likely(t1 & (sizeof(unsigned int)-1)) == 0) {
+ if (likely((t1 & (sizeof(unsigned int) - 1)) == 0)) {
t2 = src & (sizeof(unsigned int) - 1);
if (unlikely(t2 != 0)) {
=== <> ===
Tx again,
r.
ps: 1/ I also attached patches' file as compressed format
2/ I will also ask fsf to update may 'Discalimer request' to change my email address and extend it to linux tree :<)
Attachment:
beautify-memcpy.c.patch.gz
Description: GNU Zip compressed data
Attachment:
fix-likely-memcpy.c.patch.gz
Description: GNU Zip compressed data
