* Re: What's up of this if (likely()) bracing?
2008-01-05 2:13 ` Grant Grundler
@ 2008-01-05 17:44 ` rubisher
0 siblings, 0 replies; 3+ messages in thread
From: rubisher @ 2008-01-05 17:44 UTC (permalink / raw)
To: Grant Grundler; +Cc: linux-parisc
[-- Attachment #1: Type: text/plain, Size: 7467 bytes --]
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 #2: beautify-memcpy.c.patch.gz --]
[-- Type: application/x-gzip, Size: 1682 bytes --]
[-- Attachment #3: fix-likely-memcpy.c.patch.gz --]
[-- Type: application/x-gzip, Size: 299 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread