From: "H . J . Lu" <hjl@lucon.org>
Subject: Re: PATCH: Fix ll/sc for mips
Date: Thu, 31 Jan 2002 14:41:00 -0800
> On Thu, Jan 31, 2002 at 11:17:21PM +0100, Maciej W. Rozycki wrote:
> > On Thu, 31 Jan 2002, H . J . Lu wrote:
> >
> > > (__compare_and_swap): Return 0 when failed to compare or swap.
> > [...]
> > > * sysdeps/mips/atomicity.h (compare_and_swap): Return 0 when
> > > failed to compare or swap.
> >
> > Looking at the i486 implementation these are not expected to fail.
> > Unless I am missing something...
>
> Why can't it fail?
>
> static inline char
> __attribute__ ((unused))
> compare_and_swap (volatile long int *p, long int oldval, long int newval)
> {
> char ret;
> long int readval;
>
> __asm__ __volatile__ ("lock; cmpxchgl %3, %1; sete %0"
> : "=q" (ret), "=m" (*p), "=a" (readval)
> : "r" (newval), "1" (*p), "a" (oldval));
> return ret;
> }
>
> It will fail if *p is not same as oldval.
Please note that "sc" may fail even if nobody write the
variable. (See P.211 "8.4.2 Load-Linked/Sotre-Conditional" of "See
MIPS RUN" for more detail.)
So, after your patch applied, compare_and_swap() may fail, even if
*p is equal to oldval.
> >
> > > * sysdeps/unix/sysv/linux/mips/sys/tas.h (_test_and_set): Fill
> > > the delay slot.
> >
> > What's the difference? The code looks the same after changes. Also you
> > forgot to indent instructions in delay slots, which worsens readability.
>
> Are we looking at the same file? Here is the patched version:
>
> _EXTERN_INLINE int
> _test_and_set (int *p, int v) __THROW
> {
> int r, t;
>
> __asm__ __volatile__
> (".set push\n\t"
> ".set noreorder\n"
> "1:\n\t"
> "ll %0,%3\n\t"
> "beq %0,%4,2f\n\t"
> "move %1,%4\n\t"
> "sc %1,%2\n\t"
> "beqz %1,1b\n\t"
> "nop\n"
> "2:\n\t"
> ".set pop"
> : "=&r" (r), "=&r" (t), "=m" (*p)
> : "m" (*p), "r" (v)
> : "memory");
>
> return r;
> }
Gas will fill delay slots. Same object codes will be produced, so I
think you don't have to do that by hand.
---
Hiroyuki Machida
Sony Corp.
