linux-mips
[Top] [All Lists]

Re: MIPS atomic memory operations (A.K.A PR 33479).

To: David Daney <ddaney@avtrex.com>
Subject: Re: MIPS atomic memory operations (A.K.A PR 33479).
From: Thiemo Seufer <ths@networkno.de>
Date: Wed, 19 Sep 2007 09:45:15 +0100
Cc: Richard Sandiford <rsandifo@nildram.co.uk>, GCC Mailing List <gcc@gcc.gnu.org>, linux-mips@linux-mips.org
In-reply-to: <46F06980.4080500@avtrex.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <46F06980.4080500@avtrex.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.16 (2007-06-11)
David Daney wrote:
> Richard,
>
> There seems to be a small problem with the MIPS atomic memory operations 
> patch I recently committed 
> (http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01290.html), in that on a 
> dual CPU machine it does not quite work.
>
> You can look at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33479#c3 for 
> more information.
>
> Here is the code in question (from mips.h):
>
> #define MIPS_COMPARE_AND_SWAP(SUFFIX, OP)     \
>   "%(%<%[sync\n"                              \

This sync is for SB-1 implied by ll, but other MIPS systems may
need it.

>   "1:\tll" SUFFIX "\t%0,%1\n"                 \
>   "\tbne\t%0,%2,2f\n"                         \
>   "\t" OP "\t%@,%3\n"                         \
>   "\tsc" SUFFIX "\t%@,%1\n"                   \
>   "\tbeq\t%@,%.,1b\n"                         \
>   "\tnop\n"                                   \

The SB-1 needs a "sync" here.

>   "2:%]%>%)"
>
>
>
> I guess my basic question is:  Should MIPS_COMPARE_AND_SWAP have a 'sync' 
> after the 'sc'?  I would have thought that 'sc' made the write visible to 
> all CPUs, but on the SB1 it appears not to be the case.
>
> If we do need to add another 'sync' should it go in the delay slot of the 
> branch?  I would say yes because we would expect the branch to rarely 
> taken.

But it would make things a lot worse for the contended case, which is
the interesting one for performance.


Thiemo

<Prev in Thread] Current Thread [Next in Thread>