Re: PATCH: Fix ll/sc for mips

[Hiroyuki Machida <machida at sm dot sony dot co dot jp>]

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.