[PATCH] misplaced parenthesis?

[PATCH] misplaced parenthesis?

[Roel Kluin]

Fix misplaced parenthesis.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
I think this is what was intended? Note that this patch may affect
profiling.

diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
index bbda909..30207b0 100644
--- a/arch/parisc/lib/memcpy.c
+++ b/arch/parisc/lib/memcpy.c
@@ -405,7 +405,7 @@ byte_copy:
 
 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)) {

Re: [PATCH] misplaced parenthesis?

[Randolph Chung]

This is a bit confusingly written, but your patch does not appear to be 
correct.

== has higher precedence than &, so you are basically changing it to:
if (likely(t1 & 1))

it really should be

-	if (likely(t1 & (sizeof(unsigned int)-1)) == 0) {
+	if (likely((t1 & (sizeof(unsigned int)-1)) == 0)) {

randolph


Roel Kluin wrote:
> Fix misplaced parenthesis.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> I think this is what was intended? Note that this patch may affect
> profiling.
>
> diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
> index bbda909..30207b0 100644
> --- a/arch/parisc/lib/memcpy.c
> +++ b/arch/parisc/lib/memcpy.c
> @@ -405,7 +405,7 @@ byte_copy:
>  
>  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)) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: [PATCH] misplaced parenthesis?

[Matt Turner]

Does gcc produce different code for the three variations?

Matt

On Sat, May 16, 2009 at 12:14 AM, Randolph Chung <randolph@tausq.org> w=
rote:
> This is a bit confusingly written, but your patch does not appear to =
be
> correct.
>
> =3D=3D has higher precedence than &, so you are basically changing it=
 to:
> if (likely(t1 & 1))
>
> it really should be
>
> - =A0 =A0 =A0 if (likely(t1 & (sizeof(unsigned int)-1)) =3D=3D 0) {
> + =A0 =A0 =A0 if (likely((t1 & (sizeof(unsigned int)-1)) =3D=3D 0)) {
>
> randolph
>
>
> Roel Kluin wrote:
>>
>> Fix misplaced parenthesis.
>>
>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>> ---
>> I think this is what was intended? Note that this patch may affect
>> profiling.
>>
>> diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
>> index bbda909..30207b0 100644
>> --- a/arch/parisc/lib/memcpy.c
>> +++ b/arch/parisc/lib/memcpy.c
>> @@ -405,7 +405,7 @@ byte_copy:
>> =A0=A0unaligned_copy:
>> =A0 =A0 =A0 =A0/* possibly we are aligned on a word, but not on a do=
uble... */
>> - =A0 =A0 =A0 if (likely(t1 & (sizeof(unsigned int)-1)) =3D=3D 0) {
>> + =A0 =A0 =A0 if (likely(t1 & (sizeof(unsigned int)-1) =3D=3D 0)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0t2 =3D src & (sizeof(unsigned int) - =
1);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (unlikely(t2 !=3D 0)) {
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pari=
sc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-paris=
c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] misplaced parenthesis?

[Randolph Chung]

The wrong version of course produces different code....

the original and the corrected one may produce different code depending 
on how the compiler schedules things.

randolph

Matt Turner wrote:
> Does gcc produce different code for the three variations?
>
> Matt
>
> On Sat, May 16, 2009 at 12:14 AM, Randolph Chung <randolph@tausq.org> wrote:
>   
>> This is a bit confusingly written, but your patch does not appear to be
>> correct.
>>
>> == has higher precedence than &, so you are basically changing it to:
>> if (likely(t1 & 1))
>>
>> it really should be
>>
>> -       if (likely(t1 & (sizeof(unsigned int)-1)) == 0) {
>> +       if (likely((t1 & (sizeof(unsigned int)-1)) == 0)) {
>>
>> randolph
>>
>>
>> Roel Kluin wrote:
>>     
>>> Fix misplaced parenthesis.
>>>
>>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>>> ---
>>> I think this is what was intended? Note that this patch may affect
>>> profiling.
>>>
>>> diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
>>> index bbda909..30207b0 100644
>>> --- a/arch/parisc/lib/memcpy.c
>>> +++ b/arch/parisc/lib/memcpy.c
>>> @@ -405,7 +405,7 @@ byte_copy:
>>>   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)) {
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>>       
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>     
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

Re: [PATCH] misplaced parenthesis?

[Randolph Chung]

Here's an updated patch:

Reported-by: Roel Kluin <roel.kluin@gmail.com>
Signed-off-by: Randolph Chung <tausq@parisc-linux.org>

diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
index 2d68431..827e535 100644
--- a/arch/parisc/lib/memcpy.c
+++ b/arch/parisc/lib/memcpy.c
@@ -405,7 +405,7 @@ byte_copy:

 unaligned_copy:
        /* possibly we are aligned on a word, but not on a double... */
-       if (likely(t1 & (sizeof(unsigned int)-1)) =3D=3D 0) {
+       if (likely((t1 & (sizeof(unsigned int)-1)) =3D=3D 0)) {
                t2 =3D src & (sizeof(unsigned int) - 1);

                if (unlikely(t2 !=3D 0)) {


On Mon, May 18, 2009 at 9:42 AM, Randolph Chung<randolph@tausq.org> wro=
te:
> The wrong version of course produces different code....
>
> the original and the corrected one may produce different code dependi=
ng on
> how the compiler schedules things.
>
> randolph
>
> Matt Turner wrote:
>>
>> Does gcc produce different code for the three variations?
>>
>> Matt
>>
>> On Sat, May 16, 2009 at 12:14 AM, Randolph Chung <randolph@tausq.org=
>
>> wrote:
>>
>>>
>>> This is a bit confusingly written, but your patch does not appear t=
o be
>>> correct.
>>>
>>> =3D=3D has higher precedence than &, so you are basically changing =
it to:
>>> if (likely(t1 & 1))
>>>
>>> it really should be
>>>
>>> - =A0 =A0 =A0 if (likely(t1 & (sizeof(unsigned int)-1)) =3D=3D 0) {
>>> + =A0 =A0 =A0 if (likely((t1 & (sizeof(unsigned int)-1)) =3D=3D 0))=
 {
>>>
>>> randolph
>>>
>>>
>>> Roel Kluin wrote:
>>>
>>>>
>>>> Fix misplaced parenthesis.
>>>>
>>>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>>>> ---
>>>> I think this is what was intended? Note that this patch may affect
>>>> profiling.
>>>>
>>>> diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
>>>> index bbda909..30207b0 100644
>>>> --- a/arch/parisc/lib/memcpy.c
>>>> +++ b/arch/parisc/lib/memcpy.c
>>>> @@ -405,7 +405,7 @@ byte_copy:
>>>> =A0unaligned_copy:
>>>> =A0 =A0 =A0 /* possibly we are aligned on a word, but not on a dou=
ble... */
>>>> - =A0 =A0 =A0 if (likely(t1 & (sizeof(unsigned int)-1)) =3D=3D 0) =
{
>>>> + =A0 =A0 =A0 if (likely(t1 & (sizeof(unsigned int)-1) =3D=3D 0)) =
{
>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 t2 =3D src & (sizeof(unsigned int) - 1=
);
>>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (unlikely(t2 !=3D 0)) {
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-pa=
risc"
>>>> in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.ht=
ml
>>>>
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-par=
isc"
>>> in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm=
l
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pari=
sc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc"=
 in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] misplaced parenthesis?

[Andrew Morton]

On Tue, 23 Jun 2009 22:53:26 +0800
Randolph Chung <randolph@tausq.org> wrote:

> Here's an updated patch:
> 
> Reported-by: Roel Kluin <roel.kluin@gmail.com>
> Signed-off-by: Randolph Chung <tausq@parisc-linux.org>
> 
> diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
> index 2d68431..827e535 100644
> --- a/arch/parisc/lib/memcpy.c
> +++ b/arch/parisc/lib/memcpy.c
> @@ -405,7 +405,7 @@ byte_copy:
> 
>  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)) {
> 

I think I'll delete this email.  If someone has a patch which they
think should be applied, please resend it with a changelog.

Re: [PATCH] misplaced parenthesis?

[Randolph Chung]

Kyle has merged this into the parisc tree.

thanks
randolph

Andrew Morton wrote:
> On Tue, 23 Jun 2009 22:53:26 +0800
> Randolph Chung <randolph@tausq.org> wrote:
>
>   
>> Here's an updated patch:
>>
>> Reported-by: Roel Kluin <roel.kluin@gmail.com>
>> Signed-off-by: Randolph Chung <tausq@parisc-linux.org>
>>
>> diff --git a/arch/parisc/lib/memcpy.c b/arch/parisc/lib/memcpy.c
>> index 2d68431..827e535 100644
>> --- a/arch/parisc/lib/memcpy.c
>> +++ b/arch/parisc/lib/memcpy.c
>> @@ -405,7 +405,7 @@ byte_copy:
>>
>>  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)) {
>>
>>     
>
> I think I'll delete this email.  If someone has a patch which they
> think should be applied, please resend it with a changelog.
>
>