This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[Patch] MIPS: Fix PR33479 Broken atomic memory primitives.
- From: David Daney <ddaney at avtrex dot com>
- To: gcc-patches <gcc-patches at gcc dot gnu dot org>, Richard Sandiford <rsandifo at nildram dot co dot uk>
- Date: Sat, 22 Sep 2007 23:48:57 -0700
- Subject: [Patch] MIPS: Fix PR33479 Broken atomic memory primitives.
My initial commit of MIPS atomic memory primitives had a couple of
problems. The result being that the thread synchronization primitives
in libgcj would occasionally cause all threads to block forever
(technically not a deadlock, but with the same results). I think I
missed it during initial testing because the test program failed only
intermittently on a dual CPU machine, but never on the single CPU
machines I initially used for testing.
The main problem I think was that some insn constraints were missing
early-clobbers. Unfortunately a quick perusal of the assembly output of
the failing functions didn't reveal any places where the missing
early-clobber looked like it was causing bad code. However I did see
the bad code in a separate test case, so I know that it was not just a
hypothetical problem. After fixing the early-clobber problem I was
never able to get the testcase in the PR to fail, where it would fail
frequently before.
As part of trying to figure out what was going wrong, I started this
e-mail thread:
http://gcc.gnu.org/ml/gcc/2007-09/msg00448.html
Where it was pointed out that the primitives were missing a 'sync' and
that they should use a Branch Likely instead of a normal branch. I
folded both of those suggestions into this final patch.
With this patch we now use a Branch Likely unless specifically disabled
with -mno-branch-likely. Also there is a 'sync' both before and after
the atomic sequence. It was pointed out that for some CPUs a 'sync'
immediately followed by a 'll' is redundant, but I could see no concrete
documentation for this so I left it in.
Testing on mipsel-linux-gnu (with dual-core SB1 cpu).
OK to commit if no regressions found?
2007-09-22 David Daney <ddaney@avtrex.com>
PR target/33479
* config/mips/mips.md (sync_compare_and_swap<mode>, sync_old_add<mode>,
sync_new_add<mode>, sync_old_<optab><mode>, sync_new_<optab><mode>,
sync_old_nand<mode>, sync_new_nand<mode>,
sync_lock_test_and_set<mode>): Fix '&' constraint modifiers. Set
mips_branch_likely. Update length attributes.
(sync_add<mode>, sync_sub<mode>, sync_old_sub<mode>,
sync_new_sub<mode>, sync_<optab><mode>, sync_nand<mode>): Set
mips_branch_likely. Update length attributes.
* config/mips/mips.h (GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP): Define new
predicate.
(MIPS_COMPARE_AND_SWAP, MIPS_SYNC_OP, MIPS_SYNC_OLD_OP,
MIPS_SYNC_NEW_OP, MIPS_SYNC_NAND, MIPS_SYNC_OLD_NAND,
MIPS_SYNC_NEW_NAND, MIPS_SYNC_EXCHANGE): Rewrite.
* doc/invoke.texi (-mbranch-likely): Document option interaction with
atomic memory operations.
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi (revision 128552)
+++ doc/invoke.texi (working copy)
@@ -12169,8 +12169,14 @@ default for the selected architecture.
instructions may be generated if they are supported by the selected
architecture. An exception is for the MIPS32 and MIPS64 architectures
and processors which implement those architectures; for those, Branch
-Likely instructions will not be generated by default because the MIPS32
-and MIPS64 architectures specifically deprecate their use.
+Likely instructions will not be generated by default because the
+MIPS32 and MIPS64 architectures specifically deprecate their use.
+
+Unless explicitly disabled with @option{-mno-branch-likely}, Branch
+Likely instructions are always generated (for all architectures) when
+they are part of atomic memory primitives. This is done to insure
+code portability across processors that require it for proper
+operation of the atomic memory primitives.
@item -mfp-exceptions
@itemx -mno-fp-exceptions
Index: config/mips/mips.md
===================================================================
--- config/mips/mips.md (revision 128552)
+++ config/mips/mips.md (working copy)
@@ -4327,7 +4327,7 @@ (define_insn "memory_barrier"
"%|sync%-")
(define_insn "sync_compare_and_swap<mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=&d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR [(match_operand:GPR 2 "register_operand" "d,d")
@@ -4335,12 +4335,13 @@ (define_insn "sync_compare_and_swap<mode
UNSPEC_COMPARE_AND_SWAP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_COMPARE_AND_SWAP ("<d>", "li");
else
return MIPS_COMPARE_AND_SWAP ("<d>", "move");
}
- [(set_attr "length" "28")])
+ [(set_attr "length" "32")])
(define_insn "sync_add<mode>"
[(set (match_operand:GPR 0 "memory_operand" "+R,R")
@@ -4350,12 +4351,13 @@ (define_insn "sync_add<mode>"
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_OP ("<d>", "<d>addiu");
else
return MIPS_SYNC_OP ("<d>", "<d>addu");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "28")])
(define_insn "sync_sub<mode>"
[(set (match_operand:GPR 0 "memory_operand" "+R")
@@ -4365,12 +4367,13 @@ (define_insn "sync_sub<mode>"
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
- return MIPS_SYNC_OP ("<d>", "<d>subu");
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
+ return MIPS_SYNC_OP ("<d>", "<d>subu");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "28")])
(define_insn "sync_old_add<mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR
@@ -4379,12 +4382,13 @@ (define_insn "sync_old_add<mode>"
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_OLD_OP ("<d>", "<d>addiu");
else
return MIPS_SYNC_OLD_OP ("<d>", "<d>addu");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "28")])
(define_insn "sync_old_sub<mode>"
[(set (match_operand:GPR 0 "register_operand" "=&d")
@@ -4396,12 +4400,13 @@ (define_insn "sync_old_sub<mode>"
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
return MIPS_SYNC_OLD_OP ("<d>", "<d>subu");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "28")])
(define_insn "sync_new_add<mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(plus:GPR (match_operand:GPR 1 "memory_operand" "+R,R")
(match_operand:GPR 2 "arith_operand" "I,d")))
(set (match_dup 1)
@@ -4410,12 +4415,13 @@ (define_insn "sync_new_add<mode>"
UNSPEC_SYNC_NEW_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_NEW_OP ("<d>", "<d>addiu");
else
return MIPS_SYNC_NEW_OP ("<d>", "<d>addu");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "32")])
(define_insn "sync_new_sub<mode>"
[(set (match_operand:GPR 0 "register_operand" "=&d")
@@ -4427,9 +4433,10 @@ (define_insn "sync_new_sub<mode>"
UNSPEC_SYNC_NEW_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
return MIPS_SYNC_NEW_OP ("<d>", "<d>subu");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "32")])
(define_insn "sync_<optab><mode>"
[(set (match_operand:GPR 0 "memory_operand" "+R,R")
@@ -4439,15 +4446,16 @@ (define_insn "sync_<optab><mode>"
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_OP ("<d>", "<immediate_insn>");
else
return MIPS_SYNC_OP ("<d>", "<insn>");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "28")])
(define_insn "sync_old_<optab><mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR
@@ -4456,15 +4464,16 @@ (define_insn "sync_old_<optab><mode>"
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>");
else
return MIPS_SYNC_OLD_OP ("<d>", "<insn>");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "28")])
(define_insn "sync_new_<optab><mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR
@@ -4473,12 +4482,13 @@ (define_insn "sync_new_<optab><mode>"
UNSPEC_SYNC_NEW_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>");
else
return MIPS_SYNC_NEW_OP ("<d>", "<insn>");
}
- [(set_attr "length" "24")])
+ [(set_attr "length" "32")])
(define_insn "sync_nand<mode>"
[(set (match_operand:GPR 0 "memory_operand" "+R,R")
@@ -4491,46 +4501,49 @@ (define_insn "sync_nand<mode>"
else
return MIPS_SYNC_NAND ("<d>", "and");
}
- [(set_attr "length" "28")])
+ [(set_attr "length" "32")])
(define_insn "sync_old_nand<mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR [(match_operand:GPR 2 "uns_arith_operand" "K,d")]
UNSPEC_SYNC_OLD_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_OLD_NAND ("<d>", "andi");
else
return MIPS_SYNC_OLD_NAND ("<d>", "and");
}
- [(set_attr "length" "28")])
+ [(set_attr "length" "32")])
(define_insn "sync_new_nand<mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR [(match_operand:GPR 2 "uns_arith_operand" "K,d")]
UNSPEC_SYNC_NEW_OP))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_NEW_NAND ("<d>", "andi");
else
return MIPS_SYNC_NEW_NAND ("<d>", "and");
}
- [(set_attr "length" "28")])
+ [(set_attr "length" "36")])
(define_insn "sync_lock_test_and_set<mode>"
- [(set (match_operand:GPR 0 "register_operand" "=&d,d")
+ [(set (match_operand:GPR 0 "register_operand" "=d,&d")
(match_operand:GPR 1 "memory_operand" "+R,R"))
(set (match_dup 1)
(unspec_volatile:GPR [(match_operand:GPR 2 "arith_operand" "I,d")]
UNSPEC_SYNC_EXCHANGE))]
"GENERATE_LL_SC"
{
+ mips_branch_likely = GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP;
if (which_alternative == 0)
return MIPS_SYNC_EXCHANGE ("<d>", "li");
else
Index: config/mips/mips.h
===================================================================
--- config/mips/mips.h (revision 128552)
+++ config/mips/mips.h (working copy)
@@ -719,6 +719,15 @@ extern enum mips_llsc_setting mips_llsc;
&& !TARGET_SR71K \
&& !TARGET_MIPS16)
+/* True if branch likely should be used after 'sc' when generating
+ atomic memory operations with 'll' and 'sc'. Unless explicitly
+ disabled, we generate branch likely in this case because on some
+ CPUs (R10000) it is required. Doing this should allow code
+ compiled for a generic ISA to run anywhere. */
+#define GENERATE_BRANCHLIKELY_FOR_ATOMIC_OP \
+ ((target_flags_explicit & MASK_BRANCHLIKELY) == 0 \
+ || (target_flags & MASK_BRANCHLIKELY))
+
/* True if the ABI can only work with 64-bit integer registers. We
generally allow ad-hoc variations for TARGET_SINGLE_FLOAT, but
otherwise floating-point registers must also be 64-bit. */
@@ -2951,11 +2960,10 @@ while (0)
"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" \
+ "\tsc" SUFFIX "\t%@,%1\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
"\tnop\n" \
- "2:%]%>%)"
+ "2:\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -2967,10 +2975,10 @@ while (0)
"%(%<%[%|sync\n" \
"1:\tll" SUFFIX "\t%@,%0\n" \
"\t" INSN "\t%@,%@,%1\n" \
- "\tsc" SUFFIX "\t%@,%0" \
- "%-\n" \
- "\tbeq\t%@,%.,1b\n" \
- "\tnop%]%>%)"
+ "\tsc" SUFFIX "\t%@,%0\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
+ "\tnop\n" \
+ "\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -2984,10 +2992,10 @@ while (0)
"%(%<%[%|sync\n" \
"1:\tll" SUFFIX "\t%0,%1\n" \
"\t" INSN "\t%@,%0,%2\n" \
- "\tsc" SUFFIX "\t%@,%1" \
- "%-\n" \
- "\tbeq\t%@,%.,1b\n" \
- "\tnop%]%>%)"
+ "\tsc" SUFFIX "\t%@,%1\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
+ "\tnop\n" \
+ "\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -3001,10 +3009,11 @@ while (0)
"%(%<%[%|sync\n" \
"1:\tll" SUFFIX "\t%0,%1\n" \
"\t" INSN "\t%@,%0,%2\n" \
- "\tsc" SUFFIX "\t%@,%1" \
- "%-\n" \
- "\tbeq\t%@,%.,1b\n" \
- "\t" INSN "\t%0,%0,%2%]%>%)"
+ "\tsc" SUFFIX "\t%@,%1\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
+ "\tnop\n" \
+ "\t" INSN "\t%0,%0,%2\n" \
+ "\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -3018,10 +3027,10 @@ while (0)
"1:\tll" SUFFIX "\t%@,%0\n" \
"\tnor\t%@,%@,%.\n" \
"\t" INSN "\t%@,%@,%1\n" \
- "\tsc" SUFFIX "\t%@,%0" \
- "%-\n" \
- "\tbeq\t%@,%.,1b\n" \
- "\tnop%]%>%)"
+ "\tsc" SUFFIX "\t%@,%0\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
+ "\tnop\n" \
+ "\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -3037,10 +3046,10 @@ while (0)
"1:\tll" SUFFIX "\t%0,%1\n" \
"\tnor\t%@,%0,%.\n" \
"\t" INSN "\t%@,%@,%2\n" \
- "\tsc" SUFFIX "\t%@,%1" \
- "%-\n" \
- "\tbeq\t%@,%.,1b\n" \
- "\tnop%]%>%)"
+ "\tsc" SUFFIX "\t%@,%1\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
+ "\tnop\n" \
+ "\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -3056,10 +3065,11 @@ while (0)
"1:\tll" SUFFIX "\t%0,%1\n" \
"\tnor\t%0,%0,%.\n" \
"\t" INSN "\t%@,%0,%2\n" \
- "\tsc" SUFFIX "\t%@,%1" \
- "%-\n" \
- "\tbeq\t%@,%.,1b\n" \
- "\t" INSN "\t%0,%0,%2%]%>%)"
+ "\tsc" SUFFIX "\t%@,%1\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
+ "\tnop\n" \
+ "\t" INSN "\t%0,%0,%2\n" \
+ "\tsync%-%]%>%)"
/* Return an asm string that atomically:
@@ -3075,7 +3085,7 @@ while (0)
"1:\tll" SUFFIX "\t%0,%1\n" \
"\t" OP "\t%@,%2\n" \
"\tsc" SUFFIX "\t%@,%1\n" \
- "\tbeq\t%@,%.,1b\n" \
+ "\tbeq%?\t%@,%.,1b\n" \
"\tnop\n" \
"\tsync%-%]%>%)"