[Bug rtl-optimization/50065] -Os, -O2, -O3 optimization breaks LD/ST ordering on 32-bit SPARC

["tanzhangxi at gmail dot com" <gcc-bugzilla at gcc dot gnu dot org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50065

--- Comment #8 from Zhangxi Tan <tanzhangxi at gmail dot com> 2011-08-14 21:00:40 UTC ---
Thanks for the clear explanation.
I agree that a memory barrier would solve this issue.

Regarding the spinlock_unlock in linux, the regular arch_spin_unlock is
implemented with a single inline assembly. That will prevent the memory
reordering in C. However, for the 32-bit port the arch_write_unlock is still
defined as the following without a memory barrier in
arch/sparc/include/asm/spinlock_32.h

#define arch_write_unlock(rw)   do { (rw)->lock = 0; } while(0)

OTH, the 64-bit implemention is ok. Or did I miss something here.
Anyway, I think this is a separated issue from this thread.

(In reply to comment #6)
> > The code is equivalent to
> > 
> > volatile unsigned char lock;
> > int remap_barrier;
> > 
> > while (atomic_test_and_set(lock)) {
> >    while (lock) {
> >      ;
> >    }
> > }
> > remap_barrier++;
> > lock = 0;
> > 
> > Eric: could you let me know you you think the code inside function  
> > spinlock_lock(&remap_lock) is a NOP?
> 
> I don't, you simply misquoted, I wrote "the end of the code".  The first part
> of the spinlock implementation is correct, in particular you have the required
> memory barrier in spinlock_is_locked.  The second part is not correct, as you
> don't have the memory barrier in spinlock_unlock.
> 
> > Also, the arch_write_lock/unlock in the SPARC port of Linux uses a very 
> > similar implementation.
> 
> No, it precisely doesn't, it has the memory barrier in spinlock_unlock.