MIPS atomic memory operations (A.K.A PR 33479).
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" \
"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" \
"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.
Any feedback from linux-mips people is also solicited.
Thanks,
David Daney
Quoted 3 lines · David Daney
I guess my basic question is: Should
MIPS_COMPARE_AND_SWAPhave 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.
Yes, a barrier of some sort is definitely necessary. I believe the SB1 is weakly ordered, and the architecture spec permits both strong and weak ordering; but it's been a while since I tried this.
Signature · 2 lines
Daniel Jacobowitz CodeSourcery
Quoted 7 lines · David Daney
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.
Quoted 7 lines
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.
Quoted 6 lines
"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.
Quoted 11 lines
"2:%]%>%)"
I guess my basic question is: Should
MIPS_COMPARE_AND_SWAPhave 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
Quoted 10 lines · David Daney
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):
Quoted 7 lines
#define
MIPS_COMPARE_AND_SWAP(SUFFIX, OP)\
"%(%<%[sync\n" \
"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" \
Please make this loop closure branch a branch-likely. This is necessary as a errata workaround for some processors.
(I know silicon people hate me for keeping branch likely instruction alive this way but it's my job to make sure Linux and software are working without unpleasant surprises.)
Quoted 8 lines
"\tnop\n" \ "2:%]%>%)"I guess my basic question is: Should
MIPS_COMPARE_AND_SWAPhave 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.
The MIPS architecture specification specifies no memory model, so for portable code you need to make a worst case assumption which is weak ordering.
Only on R4000 and R4400 SC and SCD did imply a SYNC operation.
Quoted 3 lines
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.
Not when using a branch likely.
Btw, I recently wrote an article about memory consistency which is at
http://www.linux-mips.org/wiki/Memory_consistency. It gives a bit of an overview of things in general and on MIPS specifically. I request people with detailed knowledge of MIPS cores not specifically covered in that article to contribute.
Ralf
Quoted 2 lines · Ralf Baechle
Please make this loop closure branch a branch-likely. This is necessary as a errata workaround for some processors.
Do we emulate them for MIPS I? We do emulate "ll" and "sc" and adding
"sync" is easy (as a no-op as support for R3000 SMP is unlikely to ever happen). Adding branches-likely, hmm... Even though we do have logic to do that as a part of the FP emulator.
A workaround for a CPU erratum fits within the "-mfix-*" option family
quite well though.
Maciej
Quoted 7 lines · Maciej W. Rozycki
Quoted 2 lines · Ralf Baechle
Please make this loop closure branch a branch-likely. This is necessary as a errata workaround for some processors.
Do we emulate them for MIPS I? We do emulate "ll" and "sc" and adding
"sync" is easy
Currently, I (and thus GCC 4.3) am assuming that Linux emulates 'll', 'sc' and 'sync', If sync is not emulated, we would need to adjust the code generation so that it is not emitted on ISAs that don't support it.
Quoted 4 lines
(as a no-op as support for R3000 SMP is unlikely to ever happen). Adding branches-likely, hmm... Even though we do have logic to do that as a part of the FP emulator.
A workaround for a CPU erratum fits within the "-mfix-*" option family
quite well though.
Do we know which CPUs require branch-likely?
I would be inclined to agree with adding a "-mfix-??" option.
The only place where GCC's __sync_* primitives are generated without explicitly writing them into your program is in GCJ compiled java code that uses volatile fields.
If we expect the use of the __sync_* primitives on CPUs that require branch-likely to be rare, we shouldn't penalize those trying to rid themselves of the beasts.
Quoted 2 lines
Maciej
Quoted 3 lines · David Daney
Currently, I (and thus GCC 4.3) am assuming that Linux emulates 'll', 'sc' and 'sync', If sync is not emulated, we would need to adjust the code generation so that it is not emitted on ISAs that don't support it.
While adding "sync" is trivial enough I may have a patch ready by
tomorrow, that will not change the existing userbase and I am not entirely sure forcing such a hasty upgrade on people would be reasonable; likely not.
Quoted 4 lines
Quoted 2 lines
A workaround for a CPU erratum fits within the "-mfix-*" option family quite well though.
Do we know which CPUs require branch-likely?
The R10000; there is a note about it in <asm-mips/war.h> atR10000_LLSC_WAR.
Quoted 9 lines
I would be inclined to agree with adding a "-mfix-??" option.
The only place where GCC's __sync_* primitives are generated without explicitly writing them into your program is in GCJ compiled java code that uses volatile fields.
If we expect the use of the __sync_* primitives on CPUs that require branch-likely to be rare, we shouldn't penalize those trying to rid themselves of the beasts.
Another option is to depend on the setting of -mbranch-likely. By
default it is on only for the processors which implement it and do not discourage it, i.e. these of the MIPS II, MIPS III and MIPS IV ISAs.
Maciej
Quoted 22 lines · Maciej W. Rozycki
Quoted 3 lines · David Daney
Currently, I (and thus GCC 4.3) am assuming that Linux emulates 'll', 'sc' and 'sync', If sync is not emulated, we would need to adjust the code generation so that it is not emitted on ISAs that don't support it.
While adding "sync" is trivial enough I may have a patch ready by
tomorrow, that will not change the existing userbase and I am not entirely sure forcing such a hasty upgrade on people would be reasonable; likely not.Quoted 3 lines
Quoted 2 lines
A workaround for a CPU erratum fits within the "-mfix-*" option family quite well though.
Do we know which CPUs require branch-likely?
The R10000; there is a note about it in <
asm-mips/war.h> atR10000_LLSC_WAR.Quoted 6 lines
I would be inclined to agree with adding a "-mfix-??" option.
The only place where GCC's __sync_* primitives are generated without explicitly writing them into your program is in GCJ compiled java code that uses volatile fields.
Quoted 6 lines
Quoted 3 lines
If we expect the use of the __sync_* primitives on CPUs that require branch-likely to be rare, we shouldn't penalize those trying to rid themselves of the beasts.
Another option is to depend on the setting of -mbranch-likely. By
default it is on only for the processors which implement it and do not discourage it, i.e. these of the MIPS II, MIPS III and MIPS IV ISAs.
This seems to be the most sensible option.
I will try to work up the GCC patch tonight.
Thanks,
David Daney
Quoted 28 lines · David Daney
Quoted 27 lines · Maciej W. Rozycki
Quoted 5 lines · David Daney
Currently, I (and thus GCC 4.3) am assuming that Linux emulates 'll', 'sc' and
'sync', If sync is not emulated, we would need to adjust the code generation
so that it is not emitted on ISAs that don't support it.While adding "sync" is trivial enough I may have a patch ready by
tomorrow, that will not change the existing userbase and I am not entirely sure forcing such a hasty upgrade on people would be reasonable; likely not.Quoted 4 lines
Quoted 3 lines
A workaround for a CPU erratum fits within the "-mfix-*" option family quite
well though.Do we know which CPUs require branch-likely?
The R10000; there is a note about it in <
asm-mips/war.h> atR10000_LLSC_WAR.Quoted 11 lines
I would be inclined to agree with adding a "-mfix-??" option.
The only place where GCC's __sync_* primitives are generated without explicitly writing them into your program is in GCJ compiled java code that
uses volatile fields.If we expect the use of the __sync_* primitives on CPUs that require branch-likely to be rare, we shouldn't penalize those trying to rid themselves
of the beasts.
Quoted 7 lines
Quoted 3 lines
Another option is to depend on the setting of -mbranch-likely. By
default it is on only for the processors which implement it and do not discourage it, i.e. these of the MIPS II, MIPS III and MIPS IV ISAs.This seems to be the most sensible option.
I will try to work up the GCC patch tonight.
This means generic MIPS code (MIPS I) wil have broken atomic intrinsics when run on modern MIPS machines.
Thiemo
Quoted 3 lines · Thiemo Seufer
Quoted 3 lines
Quoted 3 lines
Another option is to depend on the setting of -mbranch-likely. By
default it is on only for the processors which implement it and do not discourage it, i.e. these of the MIPS II, MIPS III and MIPS IV ISAs.
All MIPS implementations that have branch likely also support it with good performance. So the deprecation is atm really something that has happened on paper.
The approach for LL/SC loops (where it's used for correctness) and the rest of the code where we care about code size and performance is not necessarily the same.
Quoted 6 lines
Quoted 3 lines
This seems to be the most sensible option.
I will try to work up the GCC patch tonight.
This means generic MIPS code (MIPS I) wil have broken atomic intrinsics when run on modern MIPS machines.
Oh and if it takes adding new emulations for SYNC (some pseudo MIPS II implementations lack SYNC afair) or branch likely to the kernel I will certainly support that.
Ralf
Quoted 11 lines · David Daney
Quoted 7 lines · Maciej W. Rozycki
Quoted 2 lines · Ralf Baechle
Please make this loop closure branch a branch-likely. This is necessary as a errata workaround for some processors.
Do we emulate them for MIPS I? We do emulate "ll" and "sc" and
adding "sync" is easyCurrently, I (and thus GCC 4.3) am assuming that Linux emulates 'll', 'sc' and 'sync', If sync is not emulated, we would need to adjust the code generation so that it is not emitted on ISAs that don't support it.
I just checked myself. 'sync' is not emulated. We will have to make a change so that it is not emitted on ISAs that do not support it.
David Daney
Quoted 2 lines · David Daney
I just checked myself. 'sync' is not emulated. We will have to make a change so that it is not emitted on ISAs that do not support it.
The problem is if such software is run on a newer processor it may
silently break. Tough, I know... We should have added "sync" emulation long ago. OTOH, perhaps the MIPS I userbase is not that large anymore for the emulation to be required with a short transition period only?
Maciej