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
