[PATCH] sh: Fix calculation address in atomic function

[Nobuhiro Iwamatsu]

Hi,

Thank you for your review.

2012/6/11 Kaz Kojima <kkojima@rr.iij4u.or.jp>:
> Nobuhiro Iwamatsu <iwamatsu@nigauri.org> wrote:
>> For example, in atomic_exchange_and_add, %1(__tmp) is added
>> to r2 by "add %1, r2\n\".
>> If interruption enters at this time, it will return to label 0.
>> And %1(__tmp) is added to r2 by "add %1, r2\n\", agian.
>
> Ugh, I thought that these were fixed already.  My bad.

This problem was not noticed for long time....

>
>>       * sysdeps/unix/sysv/linux/sh/bits/atomic.h (atomic_exchange_and_add):
>>       Fix calculate address by gUSA.
>
> "Fix gUSA sequence." would be an appropriate ChangeLog entry,
> since the issue is in the value when rewound, not in the address.

OK,  I wil fix this.

>
> The hunks for atomic_exchange_and_add change the semantics.
>
>> @@ -138,8 +138,8 @@ typedef uintmax_t uatomic_max_t;
>>         mov r15,r1\n\
>>         mov #-6,r15\n\
>>         0: mov.b @%2,%0\n\
>> -       add %0,%1\n\
>> -       mov.b %1,@%2\n\
>> +       add %1,%0\n\
>> +       mov.b %0,@%2\n\
>>         1: mov r1,r15"\
>>       : "=&r" (__result), "=&r" (__tmp) : rNOSP (mem), "1" (__value) \
>>       : "r0", "r1", "memory"); \
>
> atomic_exchange_and_add should return the old value of memory
> and it should be fixed like as
>
>       __asm __volatile ("\
>          mova 1f,r0\n\
>          .align 2\n\
>          mov r15,r1\n\
>          mov #(0f-1f),r15\n\
>       0: mov.b @%2,%0\n\
>          mov %1,r2\n\
>          add %0,r2\n\
>          mov.b r2,@%2\n\
>       1: mov r1,r15"\
>        : "=&r" (__result), "=&r" (__tmp) : rNOSP (mem), "1" (__value) \
>        : "r0", "r1", "r2", "memory");                 \
>

I see. I had made mistake in the understanding.
I will fix this and resend.

> The changes for the other functions look OK.
>
>>       * sysdeps/unix/sysv/linux/sh/bits/atomic.h (atomic_bit_set): Likewise.
>>       * sysdeps/unix/sysv/linux/sh/bits/atomic.h (atomic_bit_test_set): Likewise.
>
> The patch doesn't include changes for atomic_bit_set and
> atomic_bit_test_set, though it seems that atomic_bit_set doesn't
> need any changes.

Oh, sorry. This is mistake for paste..
I will fix this too.

Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6