[PATCH]: R10000 Needs LL/SC Workaround in Gcc

[PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

The attached patch adds a workaround to have gcc emit branch likely instructions 
(beqzl) in atomic operations for R10000 CPUs.  This is because revisions of this 
CPU before 3.0 misbehave, while revisions 2.6 and earlier will deadlock.  This 
issue has been noted on SGI IP28 (Indigo2 Impact R10000) systems and SGI IP27 
Origin systems.

After creating a patch to glibc based off of Debian Bug #462112 
(http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=462112), it was suggested by 
David Daney that a similar patch be created for GCC.

Feedback would be welcome on any suggestions for improving this patch (please 
CC, as I'm not subscribed to the ML).

Thanks!

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

[-- Attachment #2: gcc-trunk-r10k-beqzl.patch --]
[-- Type: text/plain, Size: 4393 bytes --]

diff -Naurp gcc.orig/gcc/config/mips/mips.h gcc/gcc/config/mips/mips.h
--- gcc.orig/gcc/config/mips/mips.h	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/mips.h	2008-10-30 23:12:11.000000000 -0400
@@ -3066,6 +3066,31 @@ while (0)
 #ifndef HAVE_AS_TLS
 #define HAVE_AS_TLS 0
 #endif
+\f
+/* Certain revisions of the R10000 Processor need an LL/SC Workaround
+   enabled.  Revisions before 3.0 misbehave on atomic operations, and
+   Revs 2.6 and lower deadlock after several seconds due to other errata.
+
+   To quote the R10K Errata:
+     Workaround: The basic idea is to inhibit the four instructions
+     from simultaneously becoming active in R10000. Padding all
+     ll/sc sequences with nops or changing the looping branch in the
+     routines to a branch likely (which is always predicted taken
+     by R10000) will work. The nops should go after the loop, and the
+     number of them should be 28. This number could be decremented for
+     each additional instruction in the ll/sc loop such as the lock
+     modifier(s) between the ll and sc, the looping branch and its
+     delay slot. For typical short routines with one ll/sc loop, any
+     instructions after the loop could also count as a decrement. The
+     nop workaround pollutes the cache more but would be a few cycles
+     faster if all the code is in the cache and the looping branch
+     is predicted not taken.  */
+
+#ifndef (_MIPS_ARCH_R10000)
+#define R10K_BEQZ_INSN "\tbeq\t%@,%.,1b\n"
+#else
+#define R10K_BEQZ_INSN "\tbeqzl\t%@,1b\n"
+#endif
 
 /* Return an asm string that atomically:
 
@@ -3083,7 +3108,7 @@ while (0)
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3108,7 +3133,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3128,7 +3153,7 @@ while (0)
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3153,7 +3178,7 @@ while (0)
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3186,7 +3211,7 @@ while (0)
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3216,7 +3241,7 @@ while (0)
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3236,7 +3261,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3253,7 +3278,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3270,7 +3295,7 @@ while (0)
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3289,7 +3314,7 @@ while (0)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3308,7 +3333,7 @@ while (0)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3326,7 +3351,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" OP "\t%@,%2\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3350,7 +3375,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Fri, 31 Oct 2008, Kumba wrote:

> Feedback would be welcome on any suggestions for improving this patch (please
> CC, as I'm not subscribed to the ML).

 Please make it run-time selectable, aggregating all the workarounds for 
the R10000 under -mfix-r10000, similarly to how it is done for other 
silicon errata.  You probably want it on by default for -march=r10000 (but 
not necessarily -march=r12000).  You need to supply a suitable 
documentation update too.

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

Kumba wrote:
> The attached patch adds a workaround to have gcc emit branch likely 
> instructions (beqzl) in atomic operations for R10000 CPUs.  This is 
> because revisions of this CPU before 3.0 misbehave, while revisions 2.6 
> and earlier will deadlock.  This issue has been noted on SGI IP28 
> (Indigo2 Impact R10000) systems and SGI IP27 Origin systems.
> 
> After creating a patch to glibc based off of Debian Bug #462112 
> (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=462112), it was 
> suggested by David Daney that a similar patch be created for GCC.
> 
> Feedback would be welcome on any suggestions for improving this patch 
> (please CC, as I'm not subscribed to the ML).
> 
> Thanks!

Oops, typo in my first patch.  Stray parenthesis around the macro check.  Fixed 
patch is included.

I'm wondering whether this should be limited to _MIPS_ARCH_R10000, though. 
Maybe _MIPS_ARCH_MIPS4 instead, because the R10000 is at minimum, a MIPS-IV CPU, 
  and there might be cases where a userland compiled with -march=mips4 could get 
used instead of one optimized for -march=r10000?

Or would MIPS-II be better, which is when the branch likely instruction was added?


-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

[-- Attachment #2: gcc-trunk-r10k-beqzl.patch --]
[-- Type: text/plain, Size: 4391 bytes --]

diff -Naurp gcc.orig/gcc/config/mips/mips.h gcc/gcc/config/mips/mips.h
--- gcc.orig/gcc/config/mips/mips.h	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/mips.h	2008-10-30 23:12:11.000000000 -0400
@@ -3066,6 +3066,31 @@ while (0)
 #ifndef HAVE_AS_TLS
 #define HAVE_AS_TLS 0
 #endif
+\f
+/* Certain revisions of the R10000 Processor need an LL/SC Workaround
+   enabled.  Revisions before 3.0 misbehave on atomic operations, and
+   Revs 2.6 and lower deadlock after several seconds due to other errata.
+
+   To quote the R10K Errata:
+     Workaround: The basic idea is to inhibit the four instructions
+     from simultaneously becoming active in R10000. Padding all
+     ll/sc sequences with nops or changing the looping branch in the
+     routines to a branch likely (which is always predicted taken
+     by R10000) will work. The nops should go after the loop, and the
+     number of them should be 28. This number could be decremented for
+     each additional instruction in the ll/sc loop such as the lock
+     modifier(s) between the ll and sc, the looping branch and its
+     delay slot. For typical short routines with one ll/sc loop, any
+     instructions after the loop could also count as a decrement. The
+     nop workaround pollutes the cache more but would be a few cycles
+     faster if all the code is in the cache and the looping branch
+     is predicted not taken.  */
+
+#ifndef _MIPS_ARCH_R10000
+#define R10K_BEQZ_INSN "\tbeq\t%@,%.,1b\n"
+#else
+#define R10K_BEQZ_INSN "\tbeqzl\t%@,1b\n"
+#endif
 
 /* Return an asm string that atomically:
 
@@ -3083,7 +3108,7 @@ while (0)
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3108,7 +3133,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3128,7 +3153,7 @@ while (0)
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3153,7 +3178,7 @@ while (0)
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3186,7 +3211,7 @@ while (0)
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3216,7 +3241,7 @@ while (0)
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3236,7 +3261,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3253,7 +3278,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3270,7 +3295,7 @@ while (0)
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3289,7 +3314,7 @@ while (0)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3308,7 +3333,7 @@ while (0)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3326,7 +3351,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" OP "\t%@,%2\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3350,7 +3375,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  R10K_BEQZ_INSN				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Kumba <kumba@gentoo.org> writes:
> Kumba wrote:
>> The attached patch adds a workaround to have gcc emit branch likely 
>> instructions (beqzl) in atomic operations for R10000 CPUs.  This is 
>> because revisions of this CPU before 3.0 misbehave, while revisions 2.6 
>> and earlier will deadlock.  This issue has been noted on SGI IP28 
>> (Indigo2 Impact R10000) systems and SGI IP27 Origin systems.
>> 
>> After creating a patch to glibc based off of Debian Bug #462112 
>> (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=462112), it was 
>> suggested by David Daney that a similar patch be created for GCC.
>> 
>> Feedback would be welcome on any suggestions for improving this patch 
>> (please CC, as I'm not subscribed to the ML).
>> 
>> Thanks!
>
> Oops, typo in my first patch.  Stray parenthesis around the macro
> check.  Fixed patch is included.
>
> I'm wondering whether this should be limited to _MIPS_ARCH_R10000,
> though.  Maybe _MIPS_ARCH_MIPS4 instead, because the R10000 is at
> minimum, a MIPS-IV CPU, and there might be cases where a userland
> compiled with -march=mips4 could get used instead of one optimized for
> -march=r10000?
>
> Or would MIPS-II be better, which is when the branch likely
> instruction was added?

As Maciej said, this should really be controlled by an -mfix-r10000
command-line option, not by the _MIPS_ARCH_* macro.  (In this context,
_MIPS_ARCH_* is a property of the compiler that you're using to build
gcc itself.)

There are two ways we could handle this:

  - Make -mfix-r10000 require -mbranch-likely.  (It mustn't _imply_
    -mbranch-likely.  It should simply check that -mbranch-likely is
    already in effect.)

  - Make -mfix-r10000 insert nops when -mbranch-likely is not in effect.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

Richard Sandiford wrote:
> 
> As Maciej said, this should really be controlled by an -mfix-r10000
> command-line option, not by the _MIPS_ARCH_* macro.  (In this context,
> _MIPS_ARCH_* is a property of the compiler that you're using to build
> gcc itself.)
> 
> There are two ways we could handle this:
> 
>   - Make -mfix-r10000 require -mbranch-likely.  (It mustn't _imply_
>     -mbranch-likely.  It should simply check that -mbranch-likely is
>     already in effect.)
> 
>   - Make -mfix-r10000 insert nops when -mbranch-likely is not in effect.

Does using -mbranch-likely change the output of those specific asm commands that 
my original patch was altering?  Or will -mfix-r10000 need to not only check the 
status of -mbranch-likely and set it if not set, but also need to modify the 
referenced beq/beqzl sets in mips.h?

If so, I assume a test for both TARGET_FIX_R10000 and TARGET_BRANCHLIKELY would 
be needed, and then if TARGET_BRANCHLIKELY doesn't exist, but TARGET_FIX_R10000 
is, insert 28 nops before beq.  Sound correct?


On setting -mbranch-likely, I found what I think is the appropriate section in 
mips.c around Line 13810:

   /* If neither -mbranch-likely nor -mno-branch-likely was given
      on the command line, set MASK_BRANCHLIKELY based on the target
      architecture and tuning flags.  Annulled delay slots are a
      size win, so we only consider the processor-specific tuning
      for !optimize_size.  */
   if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
     {
       if (ISA_HAS_BRANCHLIKELY
           && (optimize_size
               || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
         target_flags |= MASK_BRANCHLIKELY;
       else
         target_flags &= ~MASK_BRANCHLIKELY;
     }
   else if (TARGET_BRANCHLIKELY && !ISA_HAS_BRANCHLIKELY)
     warning (0, "the %qs architecture does not support branch-likely"
              " instructions", mips_arch_info->name);

I'm kind of thinking that the -mfix-r10000 setting to include -mbranch-likely 
would fit here (Assuming this is what can enable/disable that option via 
MASK_BRANCHLIKELY), but if I'm reading it right, optimizing for size disables 
brach-likely instructions.  Shouldn't -mfix-r10000 override that?

Would an equivalent conditional like this be close?:

       if (ISA_HAS_BRANCHLIKELY
           && ((optimize_size || TARGET_FIX_R10000)
               || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))


Also, does anyone have a copy of the R10000 Silicon Errata documentation kicking 
around?  Thiemo brought up a point that we may need ssnop instead of nop, but 
I'd need to check the errata for that, and that doesn't seem to exist anywhere 
anymore.  I found an old link to it on MIPS' site, but nothing else.  I've only 
got Vr10000 manuals from SGI and NEC, and they don't seem to cover 
revision-specific errata any.

Thanks!

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Kumba <kumba@gentoo.org> writes:
> Richard Sandiford wrote:
>> 
>> As Maciej said, this should really be controlled by an -mfix-r10000
>> command-line option, not by the _MIPS_ARCH_* macro.  (In this context,
>> _MIPS_ARCH_* is a property of the compiler that you're using to build
>> gcc itself.)
>> 
>> There are two ways we could handle this:
>> 
>>   - Make -mfix-r10000 require -mbranch-likely.  (It mustn't _imply_
>>     -mbranch-likely.  It should simply check that -mbranch-likely is
>>     already in effect.)
>> 
>>   - Make -mfix-r10000 insert nops when -mbranch-likely is not in effect.
>
> Does using -mbranch-likely change the output of those specific asm
> commands that my original patch was altering?

No.  In current sources, the asm templates never use branch-likely
instructions.

> Or will -mfix-r10000 need to not only check the status of
> -mbranch-likely and set it if not set, but also need to modify the
> referenced beq/beqzl sets in mips.h?

To be clear, the first option above was to check -- in mips_override_options --
that -mfix-r10000 is only used in cases where -mbranch-likely is in effect.
If we pick that option, it would be an error to use -mfix-r10000 in
other cases, and any code protected by TARGET_FIX_R10000 would be free
to use branch-likely instructions.  (Actually, we should use sorry()
instead of error() to report something like this.)

> If so, I assume a test for both TARGET_FIX_R10000 and
> TARGET_BRANCHLIKELY would be needed, and then if TARGET_BRANCHLIKELY
> doesn't exist, but TARGET_FIX_R10000 is, insert 28 nops before beq.
> Sound correct?

That's the second option above, yes.  In other words, -mfix-r10000
would support both -mbranch-likely and -mno-branch-likely, and act
accordingly.

> On setting -mbranch-likely, I found what I think is the appropriate
> section in mips.c around Line 13810:
>
>    /* If neither -mbranch-likely nor -mno-branch-likely was given
>       on the command line, set MASK_BRANCHLIKELY based on the target
>       architecture and tuning flags.  Annulled delay slots are a
>       size win, so we only consider the processor-specific tuning
>       for !optimize_size.  */
>    if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
>      {
>        if (ISA_HAS_BRANCHLIKELY
>            && (optimize_size
>                || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
>          target_flags |= MASK_BRANCHLIKELY;
>        else
>          target_flags &= ~MASK_BRANCHLIKELY;
>      }
>    else if (TARGET_BRANCHLIKELY && !ISA_HAS_BRANCHLIKELY)
>      warning (0, "the %qs architecture does not support branch-likely"
>               " instructions", mips_arch_info->name);
>
> I'm kind of thinking that the -mfix-r10000 setting to include -mbranch-likely 
> would fit here (Assuming this is what can enable/disable that option via 
> MASK_BRANCHLIKELY), but if I'm reading it right, optimizing for size disables 
> brach-likely instructions.

Well, optimize_size _enables_ branch-likely, but...

> Shouldn't -mfix-r10000 override that?

...that's a good question.  My take is "no".  I don't think we want
-mfix-r10000 to enable branch-likely instructions in cases where
it isn't necessary for R10000 errata.  If we take the first option,
we can simply raise an error if:

  TARGET_FIX_R10000
  && (target_flags_explicit & MASK_BRANCHLIKELY) == 0
      ? !ISA_HAS_BRANCH_LIKELY
      ? !TARGET_BRANCH_LIKELY)

> Also, does anyone have a copy of the R10000 Silicon Errata
> documentation kicking around?  Thiemo brought up a point that we may
> need ssnop instead of nop, but I'd need to check the errata for that,
> and that doesn't seem to exist anywhere anymore.  I found an old link
> to it on MIPS' site, but nothing else.  I've only got Vr10000 manuals
> from SGI and NEC, and they don't seem to cover revision-specific
> errata any.

Yeah, I was wondering that too.  I did a search, but couldn't
find anything.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

Richard Sandiford wrote:
> 
> To be clear, the first option above was to check -- in mips_override_options --
> that -mfix-r10000 is only used in cases where -mbranch-likely is in effect.
> If we pick that option, it would be an error to use -mfix-r10000 in
> other cases, and any code protected by TARGET_FIX_R10000 would be free
> to use branch-likely instructions.  (Actually, we should use sorry()
> instead of error() to report something like this.)

[snip]

> That's the second option above, yes.  In other words, -mfix-r10000
> would support both -mbranch-likely and -mno-branch-likely, and act
> accordingly.

So do I need to worry about modifying the asm templates at all?  Or is that only 
needed if pursuing option #2?

The branch-likely stuff is only going to work for MIPS-II or higher targets.  In 
the odd (but possible) cases where MIPS-I might be used with -mfix-r10000, I 
assume we'll still have to emit 28 nops prior to a beq/beqz instruction.  Is 
this already taken care of someplace?


> ...that's a good question.  My take is "no".  I don't think we want
> -mfix-r10000 to enable branch-likely instructions in cases where
> it isn't necessary for R10000 errata.  If we take the first option,
> we can simply raise an error if:

Hmm, okay.  Might this work to enable -mbranch-likely if -mfix-r10000?  (Kind of 
guessing by looking at other segments of code).

   if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
     {
       if (ISA_HAS_BRANCHLIKELY
           && (optimize_size
               || (!(target_flags_explicit & MASK_FIX_R10000) == 0)
               || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
         target_flags |= MASK_BRANCHLIKELY;
       else
         target_flags &= ~MASK_BRANCHLIKELY;
     }



My understanding so far for -mfix-r10000:
- Gets enabled if -march=r10000 is passed (done)
- Enable -mbranch-likely if not already enabled on >= MIPS-II (working on)
- Emits beqzl in the asm templates if enabled and >= MIPS-II (unsure)
- Emits 28 nops prior to beq/beqz if enabled and == MIPS-I (unsure)
- Ditto for asm templates (unsure)
- Documentation (not done)

Missing anything?


> Yeah, I was wondering that too.  I did a search, but couldn't
> find anything.

It seems we just need to use nop only and not worry about ssnop.


-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Kumba <kumba@gentoo.org> writes:
> Richard Sandiford wrote:
>> 
>> To be clear, the first option above was to check -- in mips_override_options --
>> that -mfix-r10000 is only used in cases where -mbranch-likely is in effect.
>> If we pick that option, it would be an error to use -mfix-r10000 in
>> other cases, and any code protected by TARGET_FIX_R10000 would be free
>> to use branch-likely instructions.  (Actually, we should use sorry()
>> instead of error() to report something like this.)
>
> [snip]
>
>> That's the second option above, yes.  In other words, -mfix-r10000
>> would support both -mbranch-likely and -mno-branch-likely, and act
>> accordingly.
>
> So do I need to worry about modifying the asm templates at all?  Or is
> that only needed if pursuing option #2?

You need to modify the asm templates whatever you do.

> The branch-likely stuff is only going to work for MIPS-II or higher
> targets.  In the odd (but possible) cases where MIPS-I might be used
> with -mfix-r10000, I assume we'll still have to emit 28 nops prior to
> a beq/beqz instruction.  Is this already taken care of someplace?

Hmm?  No, option #2 was supposed to include this work.

I feel we're talking at cross-purposes, so just to be clear:

 - In current gcc sources, the code generated by the __sync*() functions
   is not suitable for R10K processors.  That's true for all current
   command-line options.  We therefore need to add a new command-line
   option (-mfix-r10000) that selects the required behaviour.

 - As you said in your original message, there are two workarounds
   for the R10K errata:

   a) Make the asm templates use branch-likely instructions instead of
      normal branches when -mfix-r10000 is in effect.

   b) Separate loops by at least 28 instructions when -mfix-r10000 is
      in effect.

   Both workarounds require work, because gcc does neither of these
   things at present.  However, (a) is much easier to implement than (b).

   So it seems to me that there are two possible ways of implementing
   the -mfix-r10000 option:

   1) Only do (a).  Make it an error to use -mfix-r10000 when
      branch-likely instructions are not available.  We should
      check for this error in mips_override_options and use
      sorry() rather than error() to report it.

      Branch-likely instructions are not available if either:

      i) the command line includes -mno-branch-likely; or
      ii) both of the following hold:
          - the selected architecture does not provide branch-likely
            instructions; and
          - the command line does not include -mbranch-likely.

      The C condition for this is the one I gave in my previous message:

      (target_flags_explicit & MASK_BRANCHLIKELY) == 0
       ? !ISA_HAS_BRANCH_LIKELY
       ? !TARGET_BRANCH_LIKELY)

      So we should give up and call sorry() if:

          TARGET_FIX_R10000
          && (target_flags_explicit & MASK_BRANCHLIKELY) == 0
              ? !ISA_HAS_BRANCHLIKELY
              ? !TARGET_BRANCHLIKELY)

      is true.

      If we take this approach, any gcc code guarded by TARGET_FIX_R10000
      can make free use of branch-likely instructions.  No separate
      *_BRANCHLIKELY condition would be needed.

      In other words, the asm template could always use branch-likely
      instructions when TARGET_FIX_R10000 is true, and always use the
      current branch sequences otherwise.

   2) Implement both (a) and (b).  In this case, any gcc code guarded
      by TARGET_FIX_R10000 would need to check whether branch-likely
      instructions are available.  If they are, we can use either
      workaround (a) or workaroudn (b).  If they aren't, we must
      use workaround (b).

   These two options correspond to the two in my original reply.

>> ...that's a good question.  My take is "no".  I don't think we want
>> -mfix-r10000 to enable branch-likely instructions in cases where
>> it isn't necessary for R10000 errata.  If we take the first option,
>> we can simply raise an error if:
>
> Hmm, okay.  Might this work to enable -mbranch-likely if -mfix-r10000?
> (Kind of guessing by looking at other segments of code).
>
>    if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
>      {
>        if (ISA_HAS_BRANCHLIKELY
>            && (optimize_size
>                || (!(target_flags_explicit & MASK_FIX_R10000) == 0)
>                || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
>          target_flags |= MASK_BRANCHLIKELY;
>        else
>          target_flags &= ~MASK_BRANCHLIKELY;
>      }

No.  What I was trying to say in the quoted text was: -mfix-r10000
should have _no_ effect on whether branch-likely instructions are
available for general use.

As the comment above this code says:

  /* If neither -mbranch-likely nor -mno-branch-likely was given
     on the command line, set MASK_BRANCHLIKELY based on the target
     architecture and tuning flags.  Annulled delay slots are a
     size win, so we only consider the processor-specific tuning
     for !optimize_size.  */

In other words, this bit of the condition:

           (optimize_size
            || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0)

is a _tuning_ decision.  It is used in cases where both
TARGET_BRANCHLIKELY and !TARGET_BRANCH_LIKELY would be OK from
a correctness standpoint.

So suppose we only implement workaround (a) (== option (1) above).
We now need branch-likely instructions _for correctness_, but only
in certain asm templates.  It's therefore OK to override the _tuning_
decision for those asm templates: correctness trumps tuning.  But we
shouldn't change the tuning decision for _other_ branches (i.e. for
branches where -mfix-r10000 does not require a branch-likely instruction).

> My understanding so far for -mfix-r10000:
> - Gets enabled if -march=r10000 is passed (done)

Yes, provided that you never override an explicit -mfix-r10000 or
-mno-fix-r10000.

>> Yeah, I was wondering that too.  I did a search, but couldn't
>> find anything.
>
> It seems we just need to use nop only and not worry about ssnop.

Actually, I meant: I was wondering about the fact that there seems
to be no online copy of the errata sheet that describes this problem.
I've only ever seen a description of the workaround.  I've never seen
a verbatim copy of the errata itself.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

Richard Sandiford wrote:

(Quoting out of order)

> 
> I feel we're talking at cross-purposes, so just to be clear:

Not really cross-purposes.  I'm just rather new to patching something big like a 
full-blown, multi-target compiler with 20 years of history to it.  Mostly trying 
to get an appropriate understanding of your two options so I can work the logic 
out in my head and know how to go about attacking this.

This is a step up from writing a md file for processor scheduling :)


>    2) Implement both (a) and (b).  In this case, any gcc code guarded
>       by TARGET_FIX_R10000 would need to check whether branch-likely
>       instructions are available.  If they are, we can use either
>       workaround (a) or workaroudn (b).  If they aren't, we must
>       use workaround (b).

I think it's better to target this path.  While it's probably an extremely rare 
case, because this problem only affects a specific set of processor revisions, 
triggering a problem only noticed (so far) on SGI machines running Linux, I tend 
to err on the side of caution and think it's probably a good idea to do it right 
the first time.

Also, Murphy's Law.


> You need to modify the asm templates whatever you do.

This is what has me a little perplexed.  The asm templates are #define macros, 
and it's kind of dawned on me that my attempts made so far to correct the errata 
has me using preprocessor macros that are going to get translated into something 
else when gcc itself is compiled, rather than gcc changing what it outputs based 
on the flags we send it.

So I'm assuming that, poking around in the sync.md file some, the better 
approach might be to pass an extra argument to these atomic macros as they're 
evaluated in sync.md.  This extra argument being the resultant branch likely 
instruction:

	- If -mfix-r10000 isn't needed or -mbranch-likely isn't called,
	  "beq" gets passed in.
	- If -mfix-r10000 is called, and ISA_HAS_BRANCHLIKELY is false,
	  pass in 28 nops plus "beq" (is there some kind of macro that can
	  expand a single nop 28 times?).
	- If -mfix-r10000 is called and ISA_HAS_BRANCHLIKELY is true
	  and -mno-branch-likely was not called, then pass in the beqzl
	  instruction.

I think that's all the relevant combinations.  It's also probably a good idea 
too to determine the value to pass as the extra argument before the atomic macro 
is called.

Is this kind of check something that would need to be done in the md file 
directly, referencing the various macros as needed, or would it need to be 
defined as a function in mips.c and called inline in sync.md, returning a string 
value to the function as it exists?  Or is there a better way?


> Yes, provided that you never override an explicit -mfix-r10000 or
> -mno-fix-r10000.

I copied the same code for R4000 and R4400 for this:

   /* Default to working around R10000 errata only if the processor
      was selected explicitly.  */
   if ((target_flags_explicit & MASK_FIX_R10000) == 0
       && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
     target_flags |= MASK_FIX_R10000;

I assume that won't fire on r12000/r14000/r16000, right?  I know R14K isn't 
affected, but I haven't tried plugging my old R12K module back into the system 
to see what it does.  R16K is likely safe.


> Actually, I meant: I was wondering about the fact that there seems
> to be no online copy of the errata sheet that describes this problem.
> I've only ever seen a description of the workaround.  I've never seen
> a verbatim copy of the errata itself.

I tried seeing whether archive.org had anything old off of the mips.com site, 
but nothing close to the old directory structure seems to exist.  If I new what 
the PDF file name was, it might be possible to track something down on Google 
pertaining to the last publicly released revision.  Bit surprised, too, on why 
NEC doesn't have anything on necel.com.  They produced the actual silicon and 
had a hand in designing it, if I'm not mistaken.  I'd think they would at least 
have a copy if no one else.

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Kumba <kumba@gentoo.org> writes:
>>    2) Implement both (a) and (b).  In this case, any gcc code guarded
>>       by TARGET_FIX_R10000 would need to check whether branch-likely
>>       instructions are available.  If they are, we can use either
>>       workaround (a) or workaroudn (b).  If they aren't, we must
>>       use workaround (b).
>
> I think it's better to target this path.  While it's probably an
> extremely rare case, because this problem only affects a specific set
> of processor revisions, triggering a problem only noticed (so far) on
> SGI machines running Linux, I tend to err on the side of caution and
> think it's probably a good idea to do it right the first time.

Agreed, but that's just as true of option 1.  Each option is as correct
as the other.  It's just a question of whether we need the combination:

  -mips1 -mllsc -mfix-r10000

to be accepted, or whether we can treat it as a compile-time error.

If you do go for option 2, you then have to decide whether to insert
28 nops after every LL/SC loop, or whether you try to do some analysis
to avoid unnecessary nops.  The natural place for this analysis would
be mips_avoid_hazard.

>> You need to modify the asm templates whatever you do.
>
> This is what has me a little perplexed.  The asm templates are #define
> macros, and it's kind of dawned on me that my attempts made so far to
> correct the errata has me using preprocessor macros that are going to
> get translated into something else when gcc itself is compiled, rather
> than gcc changing what it outputs based on the flags we send it.
>
> So I'm assuming that, poking around in the sync.md file some, the better 
> approach might be to pass an extra argument to these atomic macros as they're 
> evaluated in sync.md.  This extra argument being the resultant branch likely 
> instruction:
>
> 	- If -mfix-r10000 isn't needed or -mbranch-likely isn't called,
> 	  "beq" gets passed in.
> 	- If -mfix-r10000 is called, and ISA_HAS_BRANCHLIKELY is false,
> 	  pass in 28 nops plus "beq" (is there some kind of macro that can
> 	  expand a single nop 28 times?).
> 	- If -mfix-r10000 is called and ISA_HAS_BRANCHLIKELY is true
> 	  and -mno-branch-likely was not called, then pass in the beqzl
> 	  instruction.
>
> I think that's all the relevant combinations.  It's also probably a
> good idea too to determine the value to pass as the extra argument
> before the atomic macro is called.

If you go for option 1, you could replace things like:

  "\tbeq\t%@,%.,1b\n"				\
  "\tnop\n"					\

with:

  "\tbeq%?\t%@,%.,1b\n"				\
  "\tnop\n"					\

and make the .md insn do:

  mips_branch_likely = TARGET_FIX_R10000;

But something nattier is needed for MIPS_SYNC_NEW_OP and MIPS_SYNC_NEW_NAND,
where the branch delay slot is not a nop.  In this case, we need to replace
things like:

  "\tbeq\t%@,%.,1b\n"				\
  "\t" INSN "\t%0,%0,%2\n"			\

with:

  "\tbeql\t%@,%.,1b\n"				\
  "\tnop\n"					\
  "\t" INSN "\t%0,%0,%2\n"			\

(INSN does not need to be executed when the branch is taken.)

I suppose adding a macro parameter would work, but it would lead
to combinatorial explosion.  I think we need to replace these
MIPS_SYNC_* macros with a function that uses output_asm_insn
to emit the loop insn-by-insn.  (This might make it possible
to factor things more than they're factored now.)

>> Yes, provided that you never override an explicit -mfix-r10000 or
>> -mno-fix-r10000.
>
> I copied the same code for R4000 and R4400 for this:
>
>    /* Default to working around R10000 errata only if the processor
>       was selected explicitly.  */
>    if ((target_flags_explicit & MASK_FIX_R10000) == 0
>        && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
>      target_flags |= MASK_FIX_R10000;

Looks good.

> I assume that won't fire on r12000/r14000/r16000, right?

Right.

>> Actually, I meant: I was wondering about the fact that there seems
>> to be no online copy of the errata sheet that describes this problem.
>> I've only ever seen a description of the workaround.  I've never seen
>> a verbatim copy of the errata itself.
>
> I tried seeing whether archive.org had anything old off of the
> mips.com site, but nothing close to the old directory structure seems
> to exist.  If I new what the PDF file name was, it might be possible
> to track something down on Google pertaining to the last publicly
> released revision.  Bit surprised, too, on why NEC doesn't have
> anything on necel.com.  They produced the actual silicon and had a
> hand in designing it, if I'm not mistaken.  I'd think they would at
> least have a copy if no one else.

Yeah.  Maybe they just want to erase bad memories ;)

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Ralf Baechle]

On Mon, Nov 03, 2008 at 08:47:25PM +0000, Richard Sandiford wrote:

> >> Actually, I meant: I was wondering about the fact that there seems
> >> to be no online copy of the errata sheet that describes this problem.
> >> I've only ever seen a description of the workaround.  I've never seen
> >> a verbatim copy of the errata itself.
> >
> > I tried seeing whether archive.org had anything old off of the
> > mips.com site, but nothing close to the old directory structure seems
> > to exist.  If I new what the PDF file name was, it might be possible
> > to track something down on Google pertaining to the last publicly
> > released revision.  Bit surprised, too, on why NEC doesn't have
> > anything on necel.com.  They produced the actual silicon and had a
> > hand in designing it, if I'm not mistaken.  I'd think they would at
> > least have a copy if no one else.
> 
> Yeah.  Maybe they just want to erase bad memories ;)

The R10000 processor is not a MIPS Technologies but SGI product.  So
you've been looking at the wrong site.  However to shorten your search,
only some of the relativly early versions of the errata sheets were
published.

  Ralf

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

Richard Sandiford wrote:
> 
> Agreed, but that's just as true of option 1.  Each option is as correct
> as the other.  It's just a question of whether we need the combination:
> 
>   -mips1 -mllsc -mfix-r10000
> 
> to be accepted, or whether we can treat it as a compile-time error.

Hmm, which do you think makes sense?  From a usage perspective, most developers 
are working in the MIPS32/MIPS64 ISA stuff.  In Gentoo, the mips port mostly 
supports SGI systems, mostly anything with a MIPS-IV capable processor (haven't 
decided on MIPS-III's fate just yet).  Debian I know has switched off of MIPS-I 
being the default for their binaries, and I think is MIPS-II now.  In all these 
cases, the target OS is usually Linux, although I know there are some Irix folks 
still out there, plus the *BSDs all got their own ports.

But I guess the question I'm pondering, is just how rare would it be for someone 
to actually need a MIPS-I binary with ll/sc and branch-likely fixes to run on 
something like an R10000?  Rare enough to justify denying them that particular 
command argument combination, and thus taking Option #1?  Or go the extra mile 
for Option #2?  I don't know if that's my call to really make, since I lack the 
statistical data to know who would be affected, and in what ways (i.e., do they 
have alternative methods, such as MIPS-II, etc..).


> If you do go for option 2, you then have to decide whether to insert
> 28 nops after every LL/SC loop, or whether you try to do some analysis
> to avoid unnecessary nops.  The natural place for this analysis would
> be mips_avoid_hazard.

Hmm, just looking at this out of curiosity to get an idea of what I might have 
to tackle, but this particular sequence looks like the key:

   /* Work out how many nops are needed.  Note that we only care about
      registers that are explicitly mentioned in the instruction's pattern.
      It doesn't matter that calls use the argument registers or that they
      clobber hi and lo.  */
   if (*hilo_delay < 2 && reg_set_p (lo_reg, pattern))
     nops = 2 - *hilo_delay;
   else if (*delayed_reg != 0 && reg_referenced_p (*delayed_reg, pattern))
     nops = 1;
   else
     nops = 0;

I'd have to do some reading around the code to get an understanding of how this 
function works and is called, but I'm taking a guess that it's just an extra 
'else if ... nops = 28 ...' for the simple approach (more complex if one were to 
try actual analysis).  Ot at minimum, another entire if block, since this does 
look like it's specifically for HI/LO checks.


> If you go for option 1, you could replace things like:
> 
>   "\tbeq\t%@,%.,1b\n"				\
>   "\tnop\n"					\
> 
> with:
> 
>   "\tbeq%?\t%@,%.,1b\n"				\
>   "\tnop\n"					\

Looks simple enough.  I found the block explaining what the %? parameter does. 
Is that in any actual documentation aside from a comment block in mips.c?  I'm 
only looking at the 4.3.2 Internals Manual, cause I don't know if 4.4.x 
Internals is online yet.  Wasn't sure if that was addressed from a documentation 
standpoint (or whether it's something that even needs to be listed online).


> and make the .md insn do:
> 
>   mips_branch_likely = TARGET_FIX_R10000;

Can this go anywhere in sync.md (i.e., at the top in a proper place), or does it 
need to go before any call to the macro templates?


> But something nattier is needed for MIPS_SYNC_NEW_OP and MIPS_SYNC_NEW_NAND,
> where the branch delay slot is not a nop.  In this case, we need to replace
> things like:
> 
>   "\tbeq\t%@,%.,1b\n"				\
>   "\t" INSN "\t%0,%0,%2\n"			\
> 
> with:
> 
>   "\tbeql\t%@,%.,1b\n"				\
>   "\tnop\n"					\
>   "\t" INSN "\t%0,%0,%2\n"			\

Looking at what %# and %/ do, Maybe a new punctuation character that simply 
dumps out a nop instead if mips_branch_likely is true?  I.e.:

     case '~':
       if (mips_branch_likely)
         fputs ("\n\tnop", file);
       break;

And:

     "\tbeq%?\t%@,%.,1b%~\n"				\
     "\t" INSN "\t%0,%0,%2\n"			\

'~' seems to be one of the last unused characters on my keyboard.  Seems like a 
good fit.


-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Ralf Baechle]

On Tue, Nov 04, 2008 at 02:14:02AM -0500, Kumba wrote:

>> Agreed, but that's just as true of option 1.  Each option is as correct
>> as the other.  It's just a question of whether we need the combination:
>>
>>   -mips1 -mllsc -mfix-r10000
>>
>> to be accepted, or whether we can treat it as a compile-time error.
>
> Hmm, which do you think makes sense?  From a usage perspective, most 

It's a crude way of asking for a generic MIPS binary that runs on anything
but works best on MIPS II+.

Makes me wonder if there is a point in having a single gcc option, something
like -march=generic which selects something like this, including all
workarounds?

  Ralf

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Tue, 4 Nov 2008, Ralf Baechle wrote:

> Makes me wonder if there is a point in having a single gcc option, something
> like -march=generic which selects something like this, including all
> workarounds?

 No, please don't.  If we decide to introduce it, someone will actually 
decide to use it wasting computing power of good machines to handle corner 
cases.  If somebody has a broken machine or a hardware vendor or a 
distributor has interest in supporting a particular flavour of breakage, 
then they are of course free to do so.  But please do not make it too easy 
to spread.  Let's give the hardware folks some incentive to fix their 
bugs. ;)

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Ralf Baechle]

On Tue, Nov 04, 2008 at 02:26:48PM +0000, Maciej W. Rozycki wrote:

> > Makes me wonder if there is a point in having a single gcc option, something
> > like -march=generic which selects something like this, including all
> > workarounds?
> 
>  No, please don't.  If we decide to introduce it, someone will actually 
> decide to use it wasting computing power of good machines to handle corner 
> cases.  If somebody has a broken machine or a hardware vendor or a 
> distributor has interest in supporting a particular flavour of breakage, 
> then they are of course free to do so.  But please do not make it too easy 
> to spread.  Let's give the hardware folks some incentive to fix their 
> bugs. ;)

I guess honorable mention for the years to come in the GCC man page
(see -mfix-two-by-two-equals-five ;-)  can do wonders.

  Ralf

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Tue, 4 Nov 2008, Kumba wrote:

> But I guess the question I'm pondering, is just how rare would it be for
> someone to actually need a MIPS-I binary with ll/sc and branch-likely fixes to
> run on something like an R10000?  Rare enough to justify denying them that
> particular command argument combination, and thus taking Option #1?  Or go the
> extra mile for Option #2?  I don't know if that's my call to really make,
> since I lack the statistical data to know who would be affected, and in what
> ways (i.e., do they have alternative methods, such as MIPS-II, etc..).

 Workarounds should be as cheap as possible maintenance-wise.  My vote is 
for requiring people in the need to use broken R10k revisions to choose a 
compatible ISA.  It actually makes sense to use 64-bit software on these 
systems implying at least MIPS III, which is also another argument not to 
try to bend backwards and support MIPS I with R10k workarounds.

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Kumba <kumba@gentoo.org> writes:
> Richard Sandiford wrote:
>> Agreed, but that's just as true of option 1.  Each option is as correct
>> as the other.  It's just a question of whether we need the combination:
>> 
>>   -mips1 -mllsc -mfix-r10000
>> 
>> to be accepted, or whether we can treat it as a compile-time error.
>
> Hmm, which do you think makes sense?  From a usage perspective, most
> developers are working in the MIPS32/MIPS64 ISA stuff.  In Gentoo, the
> mips port mostly supports SGI systems, mostly anything with a MIPS-IV
> capable processor (haven't decided on MIPS-III's fate just yet).
> Debian I know has switched off of MIPS-I being the default for their
> binaries, and I think is MIPS-II now.  In all these cases, the target
> OS is usually Linux, although I know there are some Irix folks still
> out there, plus the *BSDs all got their own ports.
>
> But I guess the question I'm pondering, is just how rare would it be
> for someone to actually need a MIPS-I binary with ll/sc and
> branch-likely fixes to run on something like an R10000?  Rare enough
> to justify denying them that particular command argument combination,
> and thus taking Option #1?  Or go the extra mile for Option #2?  I
> don't know if that's my call to really make, since I lack the
> statistical data to know who would be affected, and in what ways
> (i.e., do they have alternative methods, such as MIPS-II, etc..).

I'm not sure I have the statistical knowledge either.  I've tended
to work in embedded environments where -march=<my cpu> is almost always
the right option to use.  But like Maciej, I suspect it isn't worth
supporting the combination.  So my preference is for option #1.

You make a convincing case that the combination isn't useful for current
Linux distributions.  And it isn't useful for IRIX 6 either: the o32
system libraries are -mips2 rather than -mips1, and both GCC and
MIPSpro default to -mips2 for o32.

Also, I believe the glibc patch doesn't cope with -mips1 -mllsc either.
Is that right?  If so, that's another reason not to worry about it
for GCC.

I don't have a strong opinion though.

>> If you do go for option 2, you then have to decide whether to insert
>> 28 nops after every LL/SC loop, or whether you try to do some analysis
>> to avoid unnecessary nops.  The natural place for this analysis would
>> be mips_avoid_hazard.
>
> Hmm, just looking at this out of curiosity to get an idea of what I might have 
> to tackle, but this particular sequence looks like the key:
>
>    /* Work out how many nops are needed.  Note that we only care about
>       registers that are explicitly mentioned in the instruction's pattern.
>       It doesn't matter that calls use the argument registers or that they
>       clobber hi and lo.  */
>    if (*hilo_delay < 2 && reg_set_p (lo_reg, pattern))
>      nops = 2 - *hilo_delay;
>    else if (*delayed_reg != 0 && reg_referenced_p (*delayed_reg, pattern))
>      nops = 1;
>    else
>      nops = 0;
>
> I'd have to do some reading around the code to get an understanding of
> how this function works and is called, but I'm taking a guess that
> it's just an extra 'else if ... nops = 28 ...' for the simple approach
> (more complex if one were to try actual analysis).  Ot at minimum,
> another entire if block, since this does look like it's specifically
> for HI/LO checks.

It's a bit more complicated than that.  The current code takes advantage
of a nice property: that a gap of two instructions will avoid all the
hazards that we currently handle.  So if we come across a branch,
we only ever need to look at the branch and its delay slot; we never
need to look at the target of a branch.  For the R10000 errata,
you would either:

  (1) need to do proper global (inter-block) analaysis, which is
      a fair bit of new code; or

  (2) conservatively assume that the target of a branch might be a
      __sync_*() operation.

Also, the "nops =" part of the current code inserts "#nop" rather than
"nop" for ".set reorder" functions, because the assembler is supposed
to avoid the hazards in that case.  Unless you make GAS do the same
for this errata, you would need to:

  (1) insert a real nop instead of a hazard_nop; and

  (2) treat any asm as a potential atomic operation.

>> If you go for option 1, you could replace things like:
>> 
>>   "\tbeq\t%@,%.,1b\n"				\
>>   "\tnop\n"					\
>> 
>> with:
>> 
>>   "\tbeq%?\t%@,%.,1b\n"				\
>>   "\tnop\n"					\
>
> Looks simple enough.  I found the block explaining what the %?
> parameter does.  Is that in any actual documentation aside from a
> comment block in mips.c?  I'm only looking at the 4.3.2 Internals
> Manual, cause I don't know if 4.4.x Internals is online yet.  Wasn't
> sure if that was addressed from a documentation standpoint (or whether
> it's something that even needs to be listed online).

The internals manual intentionally doesn't cover things like this.
It's for target-independent stuff, not for internal details about
each port.  So the mips.c comment _is_ the documentation.

>> and make the .md insn do:
>> 
>>   mips_branch_likely = TARGET_FIX_R10000;
>
> Can this go anywhere in sync.md (i.e., at the top in a proper place),
> or does it need to go before any call to the macro templates?

mips_branch_likely only applies to the _current_ insn, so it needs
to go before any call the macro templates.  Please use a helper
function such as:

const char *
mips_output_sync_insn (const char *template)
{
  mips_branch_likely = TARGET_FIX_R10000;
  return template;
}

>> But something nattier is needed for MIPS_SYNC_NEW_OP and MIPS_SYNC_NEW_NAND,
>> where the branch delay slot is not a nop.  In this case, we need to replace
>> things like:
>> 
>>   "\tbeq\t%@,%.,1b\n"				\
>>   "\t" INSN "\t%0,%0,%2\n"			\
>> 
>> with:
>> 
>>   "\tbeql\t%@,%.,1b\n"				\
>>   "\tnop\n"					\
>>   "\t" INSN "\t%0,%0,%2\n"			\
>
> Looking at what %# and %/ do, Maybe a new punctuation character that simply 
> dumps out a nop instead if mips_branch_likely is true?  I.e.:
>
>      case '~':
>        if (mips_branch_likely)
>          fputs ("\n\tnop", file);
>        break;
>
> And:
>
>      "\tbeq%?\t%@,%.,1b%~\n"				\
>      "\t" INSN "\t%0,%0,%2\n"			\
>
> '~' seems to be one of the last unused characters on my keyboard.
> Seems like a good fit.

Yeah, looks good.  I'm a bit worried about running of % characters,
but like I say, we could always replace the templates with individual
output_asm_insns at some point in the future.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Markus Gothe]

[-- Attachment #1: Type: text/plain, Size: 7606 bytes --]

For what it's worth:

R10K should be able to compile with -mips4

On SGI-systems I think they deprecated all the R3K and earlier systems  
with IRIX 5.3 IIRC.

My old SGI Indigo (yeah, the original Indigo) R4K still runs fine with  
IRIX 6.5.22 and -mips3/n32-binaries last time I checked.

If in doubt ask on http://www.nekochan.net

//Markus

On 8 Nov 2008, at 10:37, Richard Sandiford wrote:

> Kumba <kumba@gentoo.org> writes:
>> Richard Sandiford wrote:
>>> Agreed, but that's just as true of option 1.  Each option is as  
>>> correct
>>> as the other.  It's just a question of whether we need the  
>>> combination:
>>>
>>>  -mips1 -mllsc -mfix-r10000
>>>
>>> to be accepted, or whether we can treat it as a compile-time error.
>>
>> Hmm, which do you think makes sense?  From a usage perspective, most
>> developers are working in the MIPS32/MIPS64 ISA stuff.  In Gentoo,  
>> the
>> mips port mostly supports SGI systems, mostly anything with a MIPS-IV
>> capable processor (haven't decided on MIPS-III's fate just yet).
>> Debian I know has switched off of MIPS-I being the default for their
>> binaries, and I think is MIPS-II now.  In all these cases, the target
>> OS is usually Linux, although I know there are some Irix folks still
>> out there, plus the *BSDs all got their own ports.
>>
>> But I guess the question I'm pondering, is just how rare would it be
>> for someone to actually need a MIPS-I binary with ll/sc and
>> branch-likely fixes to run on something like an R10000?  Rare enough
>> to justify denying them that particular command argument combination,
>> and thus taking Option #1?  Or go the extra mile for Option #2?  I
>> don't know if that's my call to really make, since I lack the
>> statistical data to know who would be affected, and in what ways
>> (i.e., do they have alternative methods, such as MIPS-II, etc..).
>
> I'm not sure I have the statistical knowledge either.  I've tended
> to work in embedded environments where -march=<my cpu> is almost  
> always
> the right option to use.  But like Maciej, I suspect it isn't worth
> supporting the combination.  So my preference is for option #1.
>
> You make a convincing case that the combination isn't useful for  
> current
> Linux distributions.  And it isn't useful for IRIX 6 either: the o32
> system libraries are -mips2 rather than -mips1, and both GCC and
> MIPSpro default to -mips2 for o32.
>
> Also, I believe the glibc patch doesn't cope with -mips1 -mllsc  
> either.
> Is that right?  If so, that's another reason not to worry about it
> for GCC.
>
> I don't have a strong opinion though.
>
>>> If you do go for option 2, you then have to decide whether to insert
>>> 28 nops after every LL/SC loop, or whether you try to do some  
>>> analysis
>>> to avoid unnecessary nops.  The natural place for this analysis  
>>> would
>>> be mips_avoid_hazard.
>>
>> Hmm, just looking at this out of curiosity to get an idea of what I  
>> might have
>> to tackle, but this particular sequence looks like the key:
>>
>>   /* Work out how many nops are needed.  Note that we only care about
>>      registers that are explicitly mentioned in the instruction's  
>> pattern.
>>      It doesn't matter that calls use the argument registers or  
>> that they
>>      clobber hi and lo.  */
>>   if (*hilo_delay < 2 && reg_set_p (lo_reg, pattern))
>>     nops = 2 - *hilo_delay;
>>   else if (*delayed_reg != 0 && reg_referenced_p (*delayed_reg,  
>> pattern))
>>     nops = 1;
>>   else
>>     nops = 0;
>>
>> I'd have to do some reading around the code to get an understanding  
>> of
>> how this function works and is called, but I'm taking a guess that
>> it's just an extra 'else if ... nops = 28 ...' for the simple  
>> approach
>> (more complex if one were to try actual analysis).  Ot at minimum,
>> another entire if block, since this does look like it's specifically
>> for HI/LO checks.
>
> It's a bit more complicated than that.  The current code takes  
> advantage
> of a nice property: that a gap of two instructions will avoid all the
> hazards that we currently handle.  So if we come across a branch,
> we only ever need to look at the branch and its delay slot; we never
> need to look at the target of a branch.  For the R10000 errata,
> you would either:
>
>  (1) need to do proper global (inter-block) analaysis, which is
>      a fair bit of new code; or
>
>  (2) conservatively assume that the target of a branch might be a
>      __sync_*() operation.
>
> Also, the "nops =" part of the current code inserts "#nop" rather than
> "nop" for ".set reorder" functions, because the assembler is supposed
> to avoid the hazards in that case.  Unless you make GAS do the same
> for this errata, you would need to:
>
>  (1) insert a real nop instead of a hazard_nop; and
>
>  (2) treat any asm as a potential atomic operation.
>
>>> If you go for option 1, you could replace things like:
>>>
>>>  "\tbeq\t%@,%.,1b\n"				\
>>>  "\tnop\n"					\
>>>
>>> with:
>>>
>>>  "\tbeq%?\t%@,%.,1b\n"				\
>>>  "\tnop\n"					\
>>
>> Looks simple enough.  I found the block explaining what the %?
>> parameter does.  Is that in any actual documentation aside from a
>> comment block in mips.c?  I'm only looking at the 4.3.2 Internals
>> Manual, cause I don't know if 4.4.x Internals is online yet.  Wasn't
>> sure if that was addressed from a documentation standpoint (or  
>> whether
>> it's something that even needs to be listed online).
>
> The internals manual intentionally doesn't cover things like this.
> It's for target-independent stuff, not for internal details about
> each port.  So the mips.c comment _is_ the documentation.
>
>>> and make the .md insn do:
>>>
>>>  mips_branch_likely = TARGET_FIX_R10000;
>>
>> Can this go anywhere in sync.md (i.e., at the top in a proper place),
>> or does it need to go before any call to the macro templates?
>
> mips_branch_likely only applies to the _current_ insn, so it needs
> to go before any call the macro templates.  Please use a helper
> function such as:
>
> const char *
> mips_output_sync_insn (const char *template)
> {
>  mips_branch_likely = TARGET_FIX_R10000;
>  return template;
> }
>
>>> But something nattier is needed for MIPS_SYNC_NEW_OP and  
>>> MIPS_SYNC_NEW_NAND,
>>> where the branch delay slot is not a nop.  In this case, we need  
>>> to replace
>>> things like:
>>>
>>>  "\tbeq\t%@,%.,1b\n"				\
>>>  "\t" INSN "\t%0,%0,%2\n"			\
>>>
>>> with:
>>>
>>>  "\tbeql\t%@,%.,1b\n"				\
>>>  "\tnop\n"					\
>>>  "\t" INSN "\t%0,%0,%2\n"			\
>>
>> Looking at what %# and %/ do, Maybe a new punctuation character  
>> that simply
>> dumps out a nop instead if mips_branch_likely is true?  I.e.:
>>
>>     case '~':
>>       if (mips_branch_likely)
>>         fputs ("\n\tnop", file);
>>       break;
>>
>> And:
>>
>>     "\tbeq%?\t%@,%.,1b%~\n"				\
>>     "\t" INSN "\t%0,%0,%2\n"			\
>>
>> '~' seems to be one of the last unused characters on my keyboard.
>> Seems like a good fit.
>
> Yeah, looks good.  I'm a bit worried about running of % characters,
> but like I say, we could always replace the templates with individual
> output_asm_insns at some point in the future.
>
> Richard
>

_______________________________________

Mr Markus Gothe
Software Engineer

Phone: +46 (0)13 21 81 20 (ext. 1046)
Fax: +46 (0)13 21 21 15
Mobile: +46 (0)70 348 44 35
Diskettgatan 11, SE-583 35 Linköping, Sweden
www.27m.com




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

[-- Attachment #1: Type: text/plain, Size: 5200 bytes --]

Richard Sandiford wrote:
> 
> I'm not sure I have the statistical knowledge either.  I've tended
> to work in embedded environments where -march=<my cpu> is almost always
> the right option to use.  But like Maciej, I suspect it isn't worth
> supporting the combination.  So my preference is for option #1.
> 
> You make a convincing case that the combination isn't useful for current
> Linux distributions.  And it isn't useful for IRIX 6 either: the o32
> system libraries are -mips2 rather than -mips1, and both GCC and
> MIPSpro default to -mips2 for o32.

Yeah, trying to handle MIPS-I stuff looks like it'll be above my head for now, 
so I'm going to aim at the second option after all.

FYI, I explain about the two different patches attached to this below.  They're 
not final by any means, but I'm doing something wrong somewhere in both them.


> Also, I believe the glibc patch doesn't cope with -mips1 -mllsc either.
> Is that right?  If so, that's another reason not to worry about it
> for GCC.

It doesn't as I coded it.  I plan on addressing that patch after the gcc-side of 
things.  Ralf suggested in that patch on libc-ports that I handle the MIPS-I 
case there, though, but if we're not going to support it in the gcc patch, then 
it probably isn't needed in the glibc patch either.  We'll see, though!


> mips_branch_likely only applies to the _current_ insn, so it needs
> to go before any call the macro templates.  Please use a helper
> function such as:
> 
> const char *
> mips_output_sync_insn (const char *template)
> {
>   mips_branch_likely = TARGET_FIX_R10000;
>   return template;
> }
> 

Done.  This is referenced in the first patch (gcc-4.4-trunk-fixr10k-z1.patch). 
The second patch (gcc-4.4-trunk-fixr10k-z2.patch) contains a form whereby I just 
re-declared mips_branch_likely and set it once-per template.  More on this below.


> Yeah, looks good.  I'm a bit worried about running of % characters,
> but like I say, we could always replace the templates with individual
> output_asm_insns at some point in the future.

Yeah, ~ is one of the last characters that doesn't seem to be completely used up 
and looks good.  That still leaves !, &, {, }, and a comma.  But those could 
look confusing with surrounding characters.




So about the two patches.  Both of these appears to accomplish the job, and 
allow gcc to begin compiling, but at one point about two hours into the build, 
genautomata will segfault when attempting to output tmp-automata.c.  I don't 
know which stage this is in...it's one of the early stages, and it's using xgcc 
at this point.

I tried running gdb on that particular invocation of genautomata, but it there's 
not much data I could gather, since the -O2 optimization removes some of the 
useful debugging info.  It segfaults at an fprintf() invocation, and 
tmp-automata.c is 0 bytes.

Here's the last few lines I get:

/usr/cvsroot/gcc/host-mips-unknown-linux-gnu/prev-gcc/xgcc 
-B/usr/cvsroot/gcc/host-mips-unknown-linux-gnu/prev-gcc/ 
-B/usr//mips-unknown-linux-gnu/bin/  -g -O2 -DIN_GCC   -W -Wall -Wwrite-strings 
-Wstrict-prototypes -Wmissing-prototypes -Wcast-qual -Wold-style-definition 
-Wc++-compat -Wmissing-format-attribute -pedantic -Wno-long-long 
-Wno-variadic-macros -Wno-overlength-strings   -DHAVE_CONFIG_H -DGENERATOR_FILE 
  -o build/genautomata \
             build/genautomata.o build/rtl.o build/read-rtl.o build/ggc-none.o 
build/vec.o build/min-insn-modes.o build/gensupport.o build/print-rtl.o 
build/errors.o ../../host-mips-unknown-linux-gnu/libiberty/libiberty.a -lm
build/genautomata ../.././gcc/config/mips/mips.md \
           insn-conditions.md > tmp-automata.c
/bin/sh: line 1: 28620 Segmentation fault      build/genautomata 
../.././gcc/config/mips/mips.md insn-conditions.md > tmp-automata.c
make[3]: *** [s-automata] Error 139
make[3]: Leaving directory `/usr/cvsroot/gcc/host-mips-unknown-linux-gnu/gcc'
make[2]: *** [all-stage2-gcc] Error 2
make[2]: Leaving directory `/usr/cvsroot/gcc'
make[1]: *** [stage2-bubble] Error 2
make[1]: Leaving directory `/usr/cvsroot/gcc'
make: *** [all] Error 2


I thought at first, it was the use of the helper function, so I backed that out 
and went with the form seen in the second patch, but that didn't help things 
either.  So I'm assuming this is related to the changes to the atomic macro 
templates, and xgcc must have something inside itself that's a little wonky. 
Not real sure how to approach this.

However, there's more.  If I rebuild genautomata by hand (using args from the 
command line), and I drop the optimization down a notch to -O1, then I can run 
the command to create tmp-automata.c, and it'll complete successfully (and the 
output in that file looks good).  So I'm a bit baffled.  I assume the issue is 
caused by my patch, unless I'm running into a regression in trunk that my patch 
simply exposes.

Is there another way to maybe extract some info on what's causing this?


Thanks!

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

[-- Attachment #2: gcc-4.4-trunk-fixr10k-z1.patch --]
[-- Type: text/plain, Size: 15235 bytes --]

diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.c gcc/gcc/config/mips/mips.c
--- gcc.orig/gcc/config/mips/mips.c	2008-11-06 00:35:19.000000000 -0500
+++ gcc/gcc/config/mips/mips.c	2008-11-09 02:10:40.000000000 -0500
@@ -6909,6 +6909,7 @@ mips_print_operand_reloc (FILE *file, rt
    '#'	Print a nop if in a ".set noreorder" block.
    '/'	Like '#', but do nothing within a delayed-branch sequence.
    '?'	Print "l" if mips_branch_likely is true
+   '~'	Print a nop if mips_branch_likely is true
    '.'	Print the name of the register with a hard-wired zero (zero or $0).
    '@'	Print the name of the assembler temporary register (at or $1).
    '^'	Print the name of the pic call-through register (t9 or $25).
@@ -6983,6 +6984,11 @@ mips_print_operand_punctuation (FILE *fi
 	putc ('l', file);
       break;
 
+    case '~':
+      if (mips_branch_likely)
+	fputs ("\n\tnop", file);
+      break;
+
     case '.':
       fputs (reg_names[GP_REG_FIRST + 0], file);
       break;
@@ -7026,7 +7032,7 @@ mips_init_print_operand_punct (void)
 {
   const char *p;
 
-  for (p = "()[]<>*#/?.@^+$|-"; *p; p++)
+  for (p = "()[]<>*#/?~.@^+$|-"; *p; p++)
     mips_print_operand_punct[(unsigned char) *p] = true;
 }
 
@@ -10250,6 +10256,17 @@ mips_output_order_conditional_branch (rt
   return mips_output_conditional_branch (insn, operands, branch[1], branch[0]);
 }
 \f
+/* Return a template for the __sync_* functions after setting mips_branch_likely
+   to the value of TARGET_FIX_R10000 to enable a proper workaround of R10000
+   errata.  */
+
+const char *
+mips_output_sync_insn (const char *template)
+{
+  mips_branch_likely = TARGET_FIX_R10000;
+  return template;
+}
+\f
 /* Return the assembly code for DIV or DDIV instruction DIVISION, which has
    the operands given by OPERANDS.  Add in a divide-by-zero check if needed.
 
@@ -13824,6 +13841,17 @@ mips_override_options (void)
     warning (0, "the %qs architecture does not support branch-likely"
 	     " instructions", mips_arch_info->name);
 
+  /* Check to see whether branch-likely instructions are not available
+     when using -mfix-r10000.  This will be true if:
+	1. -mno-branch-likely was passed.
+	2. The selected ISA does not support branch-likely and
+	   the command line does not include -mbranch-likely  */
+  if ((TARGET_FIX_R10000
+       && (target_flags_explicit & MASK_BRANCHLIKELY) == 0)
+          ? !ISA_HAS_BRANCHLIKELY
+          ? !TARGET_BRANCHLIKELY : false : false)
+    sorry ("branch-likely instructions not available");
+
   /* The effect of -mabicalls isn't defined for the EABI.  */
   if (mips_abi == ABI_EABI && TARGET_ABICALLS)
     {
@@ -13971,6 +13999,12 @@ mips_override_options (void)
       && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
     target_flags |= MASK_FIX_R4400;
 
+  /* Default to working around R10000 errata only if the processor
+     was selected explicitly.  */
+  if ((target_flags_explicit & MASK_FIX_R10000) == 0
+      && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
+    target_flags |= MASK_FIX_R10000;
+
   /* Save base state of options.  */
   mips_base_target_flags = target_flags;
   mips_base_delayed_branch = flag_delayed_branch;
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.h gcc/gcc/config/mips/mips.h
--- gcc.orig/gcc/config/mips/mips.h	2008-11-01 13:21:41.000000000 -0400
+++ gcc/gcc/config/mips/mips.h	2008-11-09 02:10:40.000000000 -0500
@@ -3083,7 +3083,7 @@ while (0)
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3108,7 +3108,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3128,7 +3128,7 @@ while (0)
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3153,7 +3153,7 @@ while (0)
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3186,7 +3186,7 @@ while (0)
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3216,7 +3216,7 @@ while (0)
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3236,7 +3236,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3253,7 +3253,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3270,7 +3270,7 @@ while (0)
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3289,7 +3289,7 @@ while (0)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3308,7 +3308,7 @@ while (0)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3326,7 +3326,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%-%]%>%)"
 
@@ -3350,7 +3350,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.opt gcc/gcc/config/mips/mips.opt
--- gcc.orig/gcc/config/mips/mips.opt	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/mips.opt	2008-11-09 02:10:40.000000000 -0500
@@ -112,6 +112,10 @@ mfix-r4400
 Target Report Mask(FIX_R4400)
 Work around certain R4400 errata
 
+mfix-r10000
+Target Report Mask(FIX_R10000)
+Work around certain R10000 errata
+
 mfix-sb1
 Target Report Var(TARGET_FIX_SB1)
 Work around errata for early SB-1 revision 2 cores
diff -Naurp -x .svn gcc.orig/gcc/config/mips/sync.md gcc/gcc/config/mips/sync.md
--- gcc.orig/gcc/config/mips/sync.md	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/sync.md	2008-11-09 02:10:40.000000000 -0500
@@ -43,9 +43,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP ("<d>", "li");
+    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP ("<d>", "li"));
   else
-    return MIPS_COMPARE_AND_SWAP ("<d>", "move");
+    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP ("<d>", "move"));
 }
   [(set_attr "length" "32")])
 
@@ -76,9 +76,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP);
+    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP));
   else
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP);
+    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP));
 }
   [(set_attr "length" "40,36")])
 
@@ -91,9 +91,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_OP ("<d>", "<d>addu");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -124,7 +124,7 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP);	
+    return mips_output_sync_insn (MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP));
 }
   [(set_attr "length" "40")])
 
@@ -160,8 +160,9 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
-				MIPS_SYNC_OLD_OP_12_NOT_NOP_REG);	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP_12 ("<insn>",
+				    MIPS_SYNC_OLD_OP_12_NOT_NOP,
+				    MIPS_SYNC_OLD_OP_12_NOT_NOP_REG));	
 }
   [(set_attr "length" "40")])
 
@@ -202,7 +203,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP);
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP_12 ("<insn>",
+				    MIPS_SYNC_NEW_OP_12_NOT_NOP));
 }
   [(set_attr "length" "40")])
 
@@ -233,7 +235,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT);	
+    return mips_output_sync_insn (MIPS_SYNC_OP_12 ("and",
+				    MIPS_SYNC_OP_12_NOT_NOT));	
 }
   [(set_attr "length" "44")])
 
@@ -267,8 +270,9 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
-				MIPS_SYNC_OLD_OP_12_NOT_NOT_REG);	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP_12 ("and",
+				    MIPS_SYNC_OLD_OP_12_NOT_NOT,
+				    MIPS_SYNC_OLD_OP_12_NOT_NOT_REG));	
 }
   [(set_attr "length" "44")])
 
@@ -307,7 +311,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT);
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP_12 ("and",
+				    MIPS_SYNC_NEW_OP_12_NOT_NOT));
 }
   [(set_attr "length" "40")])
 
@@ -319,7 +324,7 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OP ("<d>", "<d>subu");	
+  return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>subu"));	
 }
   [(set_attr "length" "28")])
 
@@ -334,9 +339,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>addiu"));	
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addu");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>addu"));	
 }
   [(set_attr "length" "28")])
 
@@ -350,7 +355,7 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OLD_OP ("<d>", "<d>subu");	
+  return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>subu"));	
 }
   [(set_attr "length" "28")])
 
@@ -365,9 +370,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>addiu"));	
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addu");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>addu"));	
 }
   [(set_attr "length" "28")])
 
@@ -381,7 +386,7 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_NEW_OP ("<d>", "<d>subu");	
+  return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>subu"));	
 }
   [(set_attr "length" "28")])
 
@@ -394,9 +399,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<immediate_insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<immediate_insn>"));	
   else
-    return MIPS_SYNC_OP ("<d>", "<insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<insn>"));	
 }
   [(set_attr "length" "28")])
 
@@ -411,9 +416,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>"));	
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<insn>"));	
 }
   [(set_attr "length" "28")])
 
@@ -428,9 +433,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>"));	
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<insn>"));	
 }
   [(set_attr "length" "28")])
 
@@ -441,9 +446,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NAND ("<d>", "andi");	
+    return mips_output_sync_insn (MIPS_SYNC_NAND ("<d>", "andi"));	
   else
-    return MIPS_SYNC_NAND ("<d>", "and");	
+    return mips_output_sync_insn (MIPS_SYNC_NAND ("<d>", "and"));	
 }
   [(set_attr "length" "32")])
 
@@ -456,9 +461,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_NAND ("<d>", "andi");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_NAND ("<d>", "andi"));	
   else
-    return MIPS_SYNC_OLD_NAND ("<d>", "and");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_NAND ("<d>", "and"));	
 }
   [(set_attr "length" "32")])
 
@@ -471,9 +476,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_NAND ("<d>", "andi");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_NAND ("<d>", "andi"));	
   else
-    return MIPS_SYNC_NEW_NAND ("<d>", "and");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_NAND ("<d>", "and"));	
 }
   [(set_attr "length" "32")])
 
@@ -486,9 +491,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE ("<d>", "li");
+    return mips_output_sync_insn (MIPS_SYNC_EXCHANGE ("<d>", "li"));
   else
-    return MIPS_SYNC_EXCHANGE ("<d>", "move");
+    return mips_output_sync_insn (MIPS_SYNC_EXCHANGE ("<d>", "move"));
 }
   [(set_attr "length" "24")])
 
@@ -516,8 +521,8 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP);
+    return mips_output_sync_insn (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP));
   else
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP);
+    return mips_output_sync_insn (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP));
 }
   [(set_attr "length" "28,24")])
diff -Naurp -x .svn gcc.orig/gcc/doc/invoke.texi gcc/gcc/doc/invoke.texi
--- gcc.orig/gcc/doc/invoke.texi	2008-10-30 22:14:29.000000000 -0400
+++ gcc/gcc/doc/invoke.texi	2008-11-03 02:15:16.000000000 -0500
@@ -666,7 +666,7 @@ Objective-C and Objective-C++ Dialects}.
 -mdivide-traps  -mdivide-breaks @gol
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
 -mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
--mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
+-mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 -mfix-r10000 -mno-fix-r10000 @gol
 -mfix-vr4120  -mno-fix-vr4120  -mfix-vr4130  -mno-fix-vr4130 @gol
 -mfix-sb1  -mno-fix-sb1 @gol
 -mflush-func=@var{func}  -mno-flush-func @gol

[-- Attachment #3: gcc-4.4-trunk-fixr10k-z2.patch --]
[-- Type: text/plain, Size: 12590 bytes --]

diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.c gcc/gcc/config/mips/mips.c
--- gcc.orig/gcc/config/mips/mips.c	2008-11-06 00:35:19.000000000 -0500
+++ gcc/gcc/config/mips/mips.c	2008-11-09 23:05:02.000000000 -0500
@@ -6909,6 +6909,7 @@ mips_print_operand_reloc (FILE *file, rt
    '#'	Print a nop if in a ".set noreorder" block.
    '/'	Like '#', but do nothing within a delayed-branch sequence.
    '?'	Print "l" if mips_branch_likely is true
+   '~'	Print a nop if mips_branch_likely is true
    '.'	Print the name of the register with a hard-wired zero (zero or $0).
    '@'	Print the name of the assembler temporary register (at or $1).
    '^'	Print the name of the pic call-through register (t9 or $25).
@@ -6983,6 +6984,11 @@ mips_print_operand_punctuation (FILE *fi
 	putc ('l', file);
       break;
 
+    case '~':
+      if (mips_branch_likely)
+	fputs ("\n\tnop", file);
+      break;
+
     case '.':
       fputs (reg_names[GP_REG_FIRST + 0], file);
       break;
@@ -7026,7 +7032,7 @@ mips_init_print_operand_punct (void)
 {
   const char *p;
 
-  for (p = "()[]<>*#/?.@^+$|-"; *p; p++)
+  for (p = "()[]<>*#/?~.@^+$|-"; *p; p++)
     mips_print_operand_punct[(unsigned char) *p] = true;
 }
 
@@ -13824,6 +13830,17 @@ mips_override_options (void)
     warning (0, "the %qs architecture does not support branch-likely"
 	     " instructions", mips_arch_info->name);
 
+  /* Check to see whether branch-likely instructions are not available
+     when using -mfix-r10000.  This will be true if:
+	1. -mno-branch-likely was passed.
+	2. The selected ISA does not support branch-likely and
+	   the command line does not include -mbranch-likely  */
+  if ((TARGET_FIX_R10000
+       && (target_flags_explicit & MASK_BRANCHLIKELY) == 0)
+          ? !ISA_HAS_BRANCHLIKELY
+          ? !TARGET_BRANCHLIKELY : false : false)
+    sorry ("branch-likely instructions not available");
+
   /* The effect of -mabicalls isn't defined for the EABI.  */
   if (mips_abi == ABI_EABI && TARGET_ABICALLS)
     {
@@ -13971,6 +13988,12 @@ mips_override_options (void)
       && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
     target_flags |= MASK_FIX_R4400;
 
+  /* Default to working around R10000 errata only if the processor
+     was selected explicitly.  */
+  if ((target_flags_explicit & MASK_FIX_R10000) == 0
+      && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
+    target_flags |= MASK_FIX_R10000;
+
   /* Save base state of options.  */
   mips_base_target_flags = target_flags;
   mips_base_delayed_branch = flag_delayed_branch;
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.h gcc/gcc/config/mips/mips.h
--- gcc.orig/gcc/config/mips/mips.h	2008-11-01 13:21:41.000000000 -0400
+++ gcc/gcc/config/mips/mips.h	2008-11-09 23:05:02.000000000 -0500
@@ -3083,7 +3083,7 @@ while (0)
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3108,7 +3108,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3128,7 +3128,7 @@ while (0)
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3153,7 +3153,7 @@ while (0)
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3186,7 +3186,7 @@ while (0)
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3216,7 +3216,7 @@ while (0)
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3236,7 +3236,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3253,7 +3253,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3270,7 +3270,7 @@ while (0)
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3289,7 +3289,7 @@ while (0)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3308,7 +3308,7 @@ while (0)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3326,7 +3326,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%-%]%>%)"
 
@@ -3350,7 +3350,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.opt gcc/gcc/config/mips/mips.opt
--- gcc.orig/gcc/config/mips/mips.opt	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/mips.opt	2008-11-09 23:05:02.000000000 -0500
@@ -112,6 +112,10 @@ mfix-r4400
 Target Report Mask(FIX_R4400)
 Work around certain R4400 errata
 
+mfix-r10000
+Target Report Mask(FIX_R10000)
+Work around certain R10000 errata
+
 mfix-sb1
 Target Report Var(TARGET_FIX_SB1)
 Work around errata for early SB-1 revision 2 cores
diff -Naurp -x .svn gcc.orig/gcc/config/mips/sync.md gcc/gcc/config/mips/sync.md
--- gcc.orig/gcc/config/mips/sync.md	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/sync.md	2008-11-09 23:05:19.000000000 -0500
@@ -42,6 +42,8 @@
 	 UNSPEC_COMPARE_AND_SWAP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_COMPARE_AND_SWAP ("<d>", "li");
   else
@@ -75,6 +77,8 @@
 			    UNSPEC_COMPARE_AND_SWAP_12))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP);
   else
@@ -90,6 +94,8 @@
 	  UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_OP ("<d>", "<d>addiu");	
   else
@@ -124,6 +130,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
+    static bool mips_branch_likely;
+    mips_branch_likely = TARGET_FIX_R10000;
     return MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP);	
 }
   [(set_attr "length" "40")])
@@ -160,6 +168,8 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
+    static bool mips_branch_likely;
+    mips_branch_likely = TARGET_FIX_R10000;
     return MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
 				MIPS_SYNC_OLD_OP_12_NOT_NOP_REG);	
 }
@@ -202,6 +212,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
+    static bool mips_branch_likely;
+    mips_branch_likely = TARGET_FIX_R10000;
     return MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP);
 }
   [(set_attr "length" "40")])
@@ -233,6 +245,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
+    static bool mips_branch_likely;
+    mips_branch_likely = TARGET_FIX_R10000;
     return MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT);	
 }
   [(set_attr "length" "44")])
@@ -267,6 +281,8 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
+    static bool mips_branch_likely;
+    mips_branch_likely = TARGET_FIX_R10000;
     return MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
 				MIPS_SYNC_OLD_OP_12_NOT_NOT_REG);	
 }
@@ -307,6 +323,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
+    static bool mips_branch_likely;
+    mips_branch_likely = TARGET_FIX_R10000;
     return MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT);
 }
   [(set_attr "length" "40")])
@@ -319,6 +337,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   return MIPS_SYNC_OP ("<d>", "<d>subu");	
 }
   [(set_attr "length" "28")])
@@ -333,6 +353,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_OLD_OP ("<d>", "<d>addiu");	
   else
@@ -350,6 +372,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   return MIPS_SYNC_OLD_OP ("<d>", "<d>subu");	
 }
   [(set_attr "length" "28")])
@@ -364,6 +388,8 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_NEW_OP ("<d>", "<d>addiu");	
   else
@@ -381,6 +407,8 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   return MIPS_SYNC_NEW_OP ("<d>", "<d>subu");	
 }
   [(set_attr "length" "28")])
@@ -393,6 +421,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_OP ("<d>", "<immediate_insn>");	
   else
@@ -410,6 +440,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>");	
   else
@@ -427,6 +459,8 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>");	
   else
@@ -440,6 +474,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_NAND ("<d>", "andi");	
   else
@@ -455,6 +491,8 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_OLD_NAND ("<d>", "andi");	
   else
@@ -470,6 +508,8 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_NEW_NAND ("<d>", "andi");	
   else
@@ -485,6 +525,8 @@
 	 UNSPEC_SYNC_EXCHANGE))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_EXCHANGE ("<d>", "li");
   else
@@ -515,6 +557,8 @@
 	  UNSPEC_SYNC_EXCHANGE_12))]
   "GENERATE_LL_SC"
 {
+  static bool mips_branch_likely;
+  mips_branch_likely = TARGET_FIX_R10000;
   if (which_alternative == 0)
     return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP);
   else
diff -Naurp -x .svn gcc.orig/gcc/doc/invoke.texi gcc/gcc/doc/invoke.texi
--- gcc.orig/gcc/doc/invoke.texi	2008-10-30 22:14:29.000000000 -0400
+++ gcc/gcc/doc/invoke.texi	2008-11-09 23:05:03.000000000 -0500
@@ -666,7 +666,7 @@ Objective-C and Objective-C++ Dialects}.
 -mdivide-traps  -mdivide-breaks @gol
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
 -mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
--mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
+-mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 -mfix-r10000 -mno-fix-r10000 @gol
 -mfix-vr4120  -mno-fix-vr4120  -mfix-vr4130  -mno-fix-vr4130 @gol
 -mfix-sb1  -mno-fix-sb1 @gol
 -mflush-func=@var{func}  -mno-flush-func @gol

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Kumba <kumba@gentoo.org> writes:
> Richard Sandiford wrote:
>> mips_branch_likely only applies to the _current_ insn, so it needs
>> to go before any call the macro templates.  Please use a helper
>> function such as:
>> 
>> const char *
>> mips_output_sync_insn (const char *template)
>> {
>>   mips_branch_likely = TARGET_FIX_R10000;
>>   return template;
>> }
>> 
>
> Done.  This is referenced in the first patch
> (gcc-4.4-trunk-fixr10k-z1.patch).  The second patch
> (gcc-4.4-trunk-fixr10k-z2.patch) contains a form whereby I just
> re-declared mips_branch_likely and set it once-per template.  More on
> this below.

The first version looks good, except for a couple of formatting
issues.  The second version doesn't work: those static mips_branch_likely
variables are local to insn-output.c, so mips.c:print_operand will never
see them.

> Yeah, ~ is one of the last characters that doesn't seem to be
> completely used up and looks good.  That still leaves !, &, {, }, and
> a comma.  But those could look confusing with surrounding characters.

Ah, well, that's not too bad.  I wouldn't have many qualms about using
%! and %& if need be.

> So about the two patches.  Both of these appears to accomplish the
> job, and allow gcc to begin compiling, but at one point about two
> hours into the build, genautomata will segfault when attempting to
> output tmp-automata.c.  I don't know which stage this is in...it's one
> of the early stages, and it's using xgcc at this point.
>
> I tried running gdb on that particular invocation of genautomata, but
> it there's not much data I could gather, since the -O2 optimization
> removes some of the useful debugging info.  It segfaults at an
> fprintf() invocation, and tmp-automata.c is 0 bytes.
>
> Here's the last few lines I get:
>
> /usr/cvsroot/gcc/host-mips-unknown-linux-gnu/prev-gcc/xgcc 
> -B/usr/cvsroot/gcc/host-mips-unknown-linux-gnu/prev-gcc/ 
> -B/usr//mips-unknown-linux-gnu/bin/  -g -O2 -DIN_GCC   -W -Wall -Wwrite-strings 
> -Wstrict-prototypes -Wmissing-prototypes -Wcast-qual -Wold-style-definition 
> -Wc++-compat -Wmissing-format-attribute -pedantic -Wno-long-long 
> -Wno-variadic-macros -Wno-overlength-strings   -DHAVE_CONFIG_H -DGENERATOR_FILE 
>   -o build/genautomata \
>              build/genautomata.o build/rtl.o build/read-rtl.o build/ggc-none.o 
> build/vec.o build/min-insn-modes.o build/gensupport.o build/print-rtl.o 
> build/errors.o ../../host-mips-unknown-linux-gnu/libiberty/libiberty.a -lm
> build/genautomata ../.././gcc/config/mips/mips.md \
>            insn-conditions.md > tmp-automata.c
> /bin/sh: line 1: 28620 Segmentation fault      build/genautomata 
> ../.././gcc/config/mips/mips.md insn-conditions.md > tmp-automata.c
> make[3]: *** [s-automata] Error 139
> make[3]: Leaving directory `/usr/cvsroot/gcc/host-mips-unknown-linux-gnu/gcc'
> make[2]: *** [all-stage2-gcc] Error 2
> make[2]: Leaving directory `/usr/cvsroot/gcc'
> make[1]: *** [stage2-bubble] Error 2
> make[1]: Leaving directory `/usr/cvsroot/gcc'
> make: *** [all] Error 2
>
>
> I thought at first, it was the use of the helper function, so I backed
> that out and went with the form seen in the second patch, but that
> didn't help things either.  So I'm assuming this is related to the
> changes to the atomic macro templates, and xgcc must have something
> inside itself that's a little wonky.  Not real sure how to approach
> this.
>
> However, there's more.  If I rebuild genautomata by hand (using args
> from the command line), and I drop the optimization down a notch to
> -O1, then I can run the command to create tmp-automata.c, and it'll
> complete successfully (and the output in that file looks good).  So
> I'm a bit baffled.  I assume the issue is caused by my patch, unless
> I'm running into a regression in trunk that my patch simply exposes.
>
> Is there another way to maybe extract some info on what's causing this?

For avoidance of doubt, I suppose the first thing to ask is: do you get
the segfault with the same checkout after you revert your patch?
It could certainly be transient breakage on trunk, like you say.

> @@ -13824,6 +13841,17 @@ mips_override_options (void)
>      warning (0, "the %qs architecture does not support branch-likely"
>  	     " instructions", mips_arch_info->name);
>  
> +  /* Check to see whether branch-likely instructions are not available
> +     when using -mfix-r10000.  This will be true if:
> +	1. -mno-branch-likely was passed.
> +	2. The selected ISA does not support branch-likely and
> +	   the command line does not include -mbranch-likely  */

Nitlet, but "to see" is redundant.  Maybe:

  /* Make sure that branch-likely instructions available when using
     -mfix-r10000.  The instructions are not available if either:

	1. -mno-branch-likely was passed.
	2. The selected ISA does not support branch-likely and
	   the command line does not include -mbranch-likely.  */

> +  if ((TARGET_FIX_R10000
> +       && (target_flags_explicit & MASK_BRANCHLIKELY) == 0)
> +          ? !ISA_HAS_BRANCHLIKELY
> +          ? !TARGET_BRANCHLIKELY : false : false)

Should just be:

  if (TARGET_FIX_R10000
      && ((target_flags_explicit & MASK_BRANCHLIKELY) == 0
          ? !ISA_HAS_BRANCHLIKELY
          : !TARGET_BRANCHLIKEL))
    sorry ("branch-likely instructions not available");

And the check should go after...

> @@ -13971,6 +13999,12 @@ mips_override_options (void)
>        && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
>      target_flags |= MASK_FIX_R4400;
>  
> +  /* Default to working around R10000 errata only if the processor
> +     was selected explicitly.  */
> +  if ((target_flags_explicit & MASK_FIX_R10000) == 0
> +      && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
> +    target_flags |= MASK_FIX_R10000;
> +
>    /* Save base state of options.  */
>    mips_base_target_flags = target_flags;
>    mips_base_delayed_branch = flag_delayed_branch;

...this.

(Which I suppose raises the question: should

  -march=r10000 -mno-branch-likely

be an error, or should it silently disable -mfix-r10000?  My vote is
for "error".  You can always write -march=r10000 -mno-branch-likely
-mno-fix-r10000 is that's really what you mean.

The suggested change -- swapping these two blocks around -- should do that.)

> @@ -76,9 +76,9 @@
>    "GENERATE_LL_SC"
>  {
>    if (which_alternative == 0)
> -    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP);
> +    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP));
>    else
> -    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP);
> +    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP));

Break lines longer than 80 chars.  Here and elsewhere, it's probably
best to use:

  return (mips_output_sync_insn
          (...stuff...));

rather than things like:

> @@ -160,8 +160,9 @@
>     (clobber (match_scratch:SI 5 "=&d"))]
>    "GENERATE_LL_SC"
>  {
> -    return MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
> -				MIPS_SYNC_OLD_OP_12_NOT_NOP_REG);	
> +    return mips_output_sync_insn (MIPS_SYNC_OLD_OP_12 ("<insn>",
> +				    MIPS_SYNC_OLD_OP_12_NOT_NOP,
> +				    MIPS_SYNC_OLD_OP_12_NOT_NOP_REG));	

...this.  Arguments should generally be indented at least as far as the
opening "(".

Looks good otherwise, thanks.  We just need to sort out the build
problem.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> For avoidance of doubt, I suppose the first thing to ask is: do you get
> the segfault with the same checkout after you revert your patch?
> It could certainly be transient breakage on trunk, like you say.

Looks like it is: PR38052.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Tue, 11 Nov 2008, Richard Sandiford wrote:

> (Which I suppose raises the question: should
> 
>   -march=r10000 -mno-branch-likely
> 
> be an error, or should it silently disable -mfix-r10000?  My vote is
> for "error".  You can always write -march=r10000 -mno-branch-likely
> -mno-fix-r10000 is that's really what you mean.
> 
> The suggested change -- swapping these two blocks around -- should do that.)

 FWIW, my preference is for an error too.  The main reason being to warn 
the user about the incompatibility of these options.

 For the opposite case a scenario where in a building system they come 
from different places each can be imagined.  Or one could be buried 
somewhere down the Makefiles in a platform-specific section of an odd 
package.  In the end the user might not be fully aware they are doing 
something wrong and the result would be broken packages would fail 
randomly on the affected processors at the run time.

 Always prefer a build error to a run-time error if possible.

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

[-- Attachment #1: Type: text/plain, Size: 4420 bytes --]

Richard Sandiford wrote:
> 
> The first version looks good, except for a couple of formatting
> issues.  The second version doesn't work: those static mips_branch_likely
> variables are local to insn-output.c, so mips.c:print_operand will never
> see them.

Yeah, I'm going to drop the second patch now that I know the genautomata thing 
is a regression of some sort.


> Nitlet, but "to see" is redundant.  Maybe:
> 
>   /* Make sure that branch-likely instructions available when using
>      -mfix-r10000.  The instructions are not available if either:
> 
> 	1. -mno-branch-likely was passed.
> 	2. The selected ISA does not support branch-likely and
> 	   the command line does not include -mbranch-likely.  */
> 

Fixed.


> Should just be:
> 
>   if (TARGET_FIX_R10000
>       && ((target_flags_explicit & MASK_BRANCHLIKELY) == 0
>           ? !ISA_HAS_BRANCHLIKELY
>           : !TARGET_BRANCHLIKEL))
>     sorry ("branch-likely instructions not available");
> 
> And the check should go after...

Fixed & Done.


> (Which I suppose raises the question: should
> 
>   -march=r10000 -mno-branch-likely
> 
> be an error, or should it silently disable -mfix-r10000?  My vote is
> for "error".  You can always write -march=r10000 -mno-branch-likely
> -mno-fix-r10000 is that's really what you mean.
> 
> The suggested change -- swapping these two blocks around -- should do that.)

Well, the check I moved around is only calling sorry(), stating that 
branch-likely isn't available.  Would additional checking be needed to 
specifically look for -march=r10000 -mno-branch-likely (and not 
-mno-fix-r10000), and then raise error() instead, warning the user about the 
invalid flag combinations (and listing them, so they don't scratch their heads 
wondering why, should such logic be buried deep in a Makefile)?  Or should the 
sorry message be made more verbose? (It looks like it completes part of a 
sentence, so I mimed the grammar I saw in other uses of it).

Also, what about cases where -march=r12000 is passed?  GCC right now has no 
technical difference between r10k through r16k (they all map to r10k for 
scheduling, per my scheduling patch), but r12k and up should have this problem 
fixed, and thus not need it.  Do we need to go that far in addressing obscure 
combinations of the flags?

Perhaps:

if ((mips_matching_cpu_name_p (mips_arch_info->name, "r12000") ||
      mips_matching_cpu_name_p (mips_arch_info->name, "r14000") ||
      mips_matching_cpu_name_p (mips_arch_info->name, "r16000"))
     && TARGET_FIX_R10000)
{
   sorry ("R10000 Errata fix not necessary on R12000 and greater CPUs");
}


> Break lines longer than 80 chars.  Here and elsewhere, it's probably
> best to use:
> 
>   return (mips_output_sync_insn
>           (...stuff...));

Done.  I used parenthesis on all the return statements, even if they stayed on 
one line, for consistency.  Any qualms with that?  Some of the lines are just 
shy of the 80-char limit by 2-4 chars, so having the parans there I suppose can 
preempt any future changes that might necessitate a newline + indentation being 
added.


> Looks good otherwise, thanks.  We just need to sort out the build
> problem.

I added to that bug you mentioned (in another mail), as I determined that the 
problem flag is -foptimize-sibling-calls combined with -O1.  With -O0, it will 
run just fine, so there's another one or more flags enabled by -O1 that are 
causing the fluke.  No idea if it's genautomata itself, since I peeked into SVN 
and that source file hasn't seen any activity in two months.  I figure it must 
be one of the other files that get linked in.

I'm running a build now on my x86 box to see if I hit the same error there, or 
if this is limited only to mips (the bugs suggests it might, but one never knows).


Also, what about a test case?  This is pretty dangerous on Rev 2.5 R10Ks, 
potentially locking them up, so I don't know if a testcase would be necessary. 
No idea if there is a "safer" way to check for this flaw and recover from it. 
The below link is the snippet of code that reliably hung an IP28 machine:

http://www.linux-mips.org/archives/linux-mips/2008-01/msg00186.html


Thanks!

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

[-- Attachment #2: gcc-4.4-trunk-fixr10k-z3.patch --]
[-- Type: text/plain, Size: 15022 bytes --]

diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.c gcc/gcc/config/mips/mips.c
--- gcc.orig/gcc/config/mips/mips.c	2008-11-06 00:35:19.000000000 -0500
+++ gcc/gcc/config/mips/mips.c	2008-11-11 22:44:27.000000000 -0500
@@ -6909,6 +6909,7 @@ mips_print_operand_reloc (FILE *file, rt
    '#'	Print a nop if in a ".set noreorder" block.
    '/'	Like '#', but do nothing within a delayed-branch sequence.
    '?'	Print "l" if mips_branch_likely is true
+   '~'	Print a nop if mips_branch_likely is true
    '.'	Print the name of the register with a hard-wired zero (zero or $0).
    '@'	Print the name of the assembler temporary register (at or $1).
    '^'	Print the name of the pic call-through register (t9 or $25).
@@ -6983,6 +6984,11 @@ mips_print_operand_punctuation (FILE *fi
 	putc ('l', file);
       break;
 
+    case '~':
+      if (mips_branch_likely)
+	fputs ("\n\tnop", file);
+      break;
+
     case '.':
       fputs (reg_names[GP_REG_FIRST + 0], file);
       break;
@@ -7026,7 +7032,7 @@ mips_init_print_operand_punct (void)
 {
   const char *p;
 
-  for (p = "()[]<>*#/?.@^+$|-"; *p; p++)
+  for (p = "()[]<>*#/?~.@^+$|-"; *p; p++)
     mips_print_operand_punct[(unsigned char) *p] = true;
 }
 
@@ -10250,6 +10256,17 @@ mips_output_order_conditional_branch (rt
   return mips_output_conditional_branch (insn, operands, branch[1], branch[0]);
 }
 \f
+/* Return a template for the __sync_* functions after setting mips_branch_likely
+   to the value of TARGET_FIX_R10000 to enable a proper workaround of R10000
+   errata.  */
+
+const char *
+mips_output_sync_insn (const char *template)
+{
+  mips_branch_likely = TARGET_FIX_R10000;
+  return template;
+}
+\f
 /* Return the assembly code for DIV or DDIV instruction DIVISION, which has
    the operands given by OPERANDS.  Add in a divide-by-zero check if needed.
 
@@ -13971,6 +13988,23 @@ mips_override_options (void)
       && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
     target_flags |= MASK_FIX_R4400;
 
+  /* Default to working around R10000 errata only if the processor
+     was selected explicitly.  */
+  if ((target_flags_explicit & MASK_FIX_R10000) == 0
+      && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
+    target_flags |= MASK_FIX_R10000;
+
+  /* Make sure that branch-likely instructions available when using
+     -mfix-r10000.  The instructions are not available if either:
+	1. -mno-branch-likely was passed.
+	2. The selected ISA does not support branch-likely and
+	   the command line does not include -mbranch-likely  */
+  if (TARGET_FIX_R10000
+      && ((target_flags_explicit & MASK_BRANCHLIKELY) == 0
+          ? !ISA_HAS_BRANCHLIKELY
+          : !TARGET_BRANCHLIKEL))
+    sorry ("branch-likely instructions not available");
+
   /* Save base state of options.  */
   mips_base_target_flags = target_flags;
   mips_base_delayed_branch = flag_delayed_branch;
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.h gcc/gcc/config/mips/mips.h
--- gcc.orig/gcc/config/mips/mips.h	2008-11-11 22:33:57.000000000 -0500
+++ gcc/gcc/config/mips/mips.h	2008-11-11 22:38:37.000000000 -0500
@@ -3089,7 +3089,7 @@ while (0)
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3114,7 +3114,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3134,7 +3134,7 @@ while (0)
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3159,7 +3159,7 @@ while (0)
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3192,7 +3192,7 @@ while (0)
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3222,7 +3222,7 @@ while (0)
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3242,7 +3242,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3259,7 +3259,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3276,7 +3276,7 @@ while (0)
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3295,7 +3295,7 @@ while (0)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3314,7 +3314,7 @@ while (0)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3332,7 +3332,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%-%]%>%)"
 
@@ -3356,7 +3356,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.opt gcc/gcc/config/mips/mips.opt
--- gcc.orig/gcc/config/mips/mips.opt	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/mips.opt	2008-11-11 22:38:37.000000000 -0500
@@ -112,6 +112,10 @@ mfix-r4400
 Target Report Mask(FIX_R4400)
 Work around certain R4400 errata
 
+mfix-r10000
+Target Report Mask(FIX_R10000)
+Work around certain R10000 errata
+
 mfix-sb1
 Target Report Var(TARGET_FIX_SB1)
 Work around errata for early SB-1 revision 2 cores
diff -Naurp -x .svn gcc.orig/gcc/config/mips/sync.md gcc/gcc/config/mips/sync.md
--- gcc.orig/gcc/config/mips/sync.md	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/sync.md	2008-11-11 22:58:06.000000000 -0500
@@ -43,9 +43,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP ("<d>", "li");
+    return (mips_output_sync_insn (MIPS_COMPARE_AND_SWAP ("<d>", "li")));
   else
-    return MIPS_COMPARE_AND_SWAP ("<d>", "move");
+    return (mips_output_sync_insn (MIPS_COMPARE_AND_SWAP ("<d>", "move")));
 }
   [(set_attr "length" "32")])
 
@@ -76,9 +76,11 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP)));
   else
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP)));
 }
   [(set_attr "length" "40,36")])
 
@@ -91,9 +93,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<d>addiu");	
+    return (mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>addiu")));
   else
-    return MIPS_SYNC_OP ("<d>", "<d>addu");	
+    return (mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>addu")));
 }
   [(set_attr "length" "28")])
 
@@ -124,7 +126,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP)));
 }
   [(set_attr "length" "40")])
 
@@ -160,8 +163,9 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
-				MIPS_SYNC_OLD_OP_12_NOT_NOP_REG);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
+	     MIPS_SYNC_OLD_OP_12_NOT_NOP_REG)));
 }
   [(set_attr "length" "40")])
 
@@ -202,7 +206,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP)));
 }
   [(set_attr "length" "40")])
 
@@ -233,7 +238,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT)));
 }
   [(set_attr "length" "44")])
 
@@ -267,8 +273,9 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
-				MIPS_SYNC_OLD_OP_12_NOT_NOT_REG);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
+	     MIPS_SYNC_OLD_OP_12_NOT_NOT_REG)));
 }
   [(set_attr "length" "44")])
 
@@ -307,7 +314,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT)));
 }
   [(set_attr "length" "40")])
 
@@ -319,7 +327,7 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OP ("<d>", "<d>subu");	
+  return (mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>subu")));
 }
   [(set_attr "length" "28")])
 
@@ -334,9 +342,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addiu");	
+    return (mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>addiu")));
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addu");	
+    return (mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>addu")));
 }
   [(set_attr "length" "28")])
 
@@ -350,7 +358,7 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OLD_OP ("<d>", "<d>subu");	
+  return (mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>subu")));
 }
   [(set_attr "length" "28")])
 
@@ -365,9 +373,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addiu");	
+    return (mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>addiu")));
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addu");	
+    return (mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>addu")));
 }
   [(set_attr "length" "28")])
 
@@ -381,7 +389,7 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_NEW_OP ("<d>", "<d>subu");	
+  return (mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>subu")));
 }
   [(set_attr "length" "28")])
 
@@ -394,9 +402,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_OP ("<d>", "<insn>");	
+    return (mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<insn>")));
 }
   [(set_attr "length" "28")])
 
@@ -411,9 +419,11 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP ("<d>", "<insn>")));
 }
   [(set_attr "length" "28")])
 
@@ -428,9 +438,11 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP ("<d>", "<insn>")));
 }
   [(set_attr "length" "28")])
 
@@ -441,9 +453,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NAND ("<d>", "andi");	
+    return (mips_output_sync_insn (MIPS_SYNC_NAND ("<d>", "andi")));
   else
-    return MIPS_SYNC_NAND ("<d>", "and");	
+    return (mips_output_sync_insn (MIPS_SYNC_NAND ("<d>", "and")));
 }
   [(set_attr "length" "32")])
 
@@ -456,9 +468,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_NAND ("<d>", "andi");	
+    return (mips_output_sync_insn (MIPS_SYNC_OLD_NAND ("<d>", "andi")));
   else
-    return MIPS_SYNC_OLD_NAND ("<d>", "and");	
+    return (mips_output_sync_insn (MIPS_SYNC_OLD_NAND ("<d>", "and")));
 }
   [(set_attr "length" "32")])
 
@@ -471,9 +483,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_NAND ("<d>", "andi");	
+    return (mips_output_sync_insn (MIPS_SYNC_NEW_NAND ("<d>", "andi")));
   else
-    return MIPS_SYNC_NEW_NAND ("<d>", "and");	
+    return (mips_output_sync_insn (MIPS_SYNC_NEW_NAND ("<d>", "and")));
 }
   [(set_attr "length" "32")])
 
@@ -486,9 +498,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE ("<d>", "li");
+    return (mips_output_sync_insn (MIPS_SYNC_EXCHANGE ("<d>", "li")));
   else
-    return MIPS_SYNC_EXCHANGE ("<d>", "move");
+    return (mips_output_sync_insn (MIPS_SYNC_EXCHANGE ("<d>", "move")));
 }
   [(set_attr "length" "24")])
 
@@ -516,8 +528,10 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP)));
   else
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP)));
 }
   [(set_attr "length" "28,24")])
diff -Naurp -x .svn gcc.orig/gcc/doc/invoke.texi gcc/gcc/doc/invoke.texi
--- gcc.orig/gcc/doc/invoke.texi	2008-10-30 22:14:29.000000000 -0400
+++ gcc/gcc/doc/invoke.texi	2008-11-11 22:38:37.000000000 -0500
@@ -666,7 +666,7 @@ Objective-C and Objective-C++ Dialects}.
 -mdivide-traps  -mdivide-breaks @gol
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
 -mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
--mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
+-mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 -mfix-r10000 -mno-fix-r10000 @gol
 -mfix-vr4120  -mno-fix-vr4120  -mfix-vr4130  -mno-fix-vr4130 @gol
 -mfix-sb1  -mno-fix-sb1 @gol
 -mflush-func=@var{func}  -mno-flush-func @gol

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Thanks for the new patch.  Generally looks good.

Kumba <kumba@gentoo.org> writes:
> Richard Sandiford wrote:
>> (Which I suppose raises the question: should
>> 
>>   -march=r10000 -mno-branch-likely
>> 
>> be an error, or should it silently disable -mfix-r10000?  My vote is
>> for "error".  You can always write -march=r10000 -mno-branch-likely
>> -mno-fix-r10000 is that's really what you mean.
>> 
>> The suggested change -- swapping these two blocks around -- should do that.)
>
> Well, the check I moved around is only calling sorry(), stating that 
> branch-likely isn't available.  Would additional checking be needed to 
> specifically look for -march=r10000 -mno-branch-likely (and not 
> -mno-fix-r10000), and then raise error() instead, warning the user about the 
> invalid flag combinations

It should be sorry() in all cases.

> (and listing them, so they don't scratch their heads wondering why,
> should such logic be buried deep in a Makefile)?  Or should the sorry
> message be made more verbose? (It looks like it completes part of a
> sentence, so I mimed the grammar I saw in other uses of it).

Well, I suppose the current sorry() is too terse even for an
explicit -mfix-r10000.  How about:

  sorry ("%qs requires branch-likely instructions\n", "-mfix-r10000");

(Done that way so that we can reuse any translations in cases where
we need branch-likely instructions for some other option.)  I think
that's OK even for "-march=r10000 -mno-branch-likely"; the documentation
should make it clear that -mfix-r10000 is the default for -march=r10000.

> Also, what about cases where -march=r12000 is passed?  GCC right now
> has no technical difference between r10k through r16k (they all map to
> r10k for scheduling, per my scheduling patch), but r12k and up should
> have this problem fixed, and thus not need it.  Do we need to go that
> far in addressing obscure combinations of the flags?
>
> Perhaps:
>
> if ((mips_matching_cpu_name_p (mips_arch_info->name, "r12000") ||
>       mips_matching_cpu_name_p (mips_arch_info->name, "r14000") ||
>       mips_matching_cpu_name_p (mips_arch_info->name, "r16000"))
>      && TARGET_FIX_R10000)
> {
>    sorry ("R10000 Errata fix not necessary on R12000 and greater CPUs");
> }

No, there's no need for anything like this.

>> Break lines longer than 80 chars.  Here and elsewhere, it's probably
>> best to use:
>> 
>>   return (mips_output_sync_insn
>>           (...stuff...));
>
> Done.  I used parenthesis on all the return statements, even if they stayed on 
> one line, for consistency.  Any qualms with that?

'Fraid so. ;)  You had those right the first time.

> Some of the lines are just shy of the 80-char limit by 2-4 chars, so
> having the parans there I suppose can preempt any future changes that
> might necessitate a newline + indentation being added.

Whoever gets to make such changes also gets to format the new version
correctly.  There's no need to preempt them by adding redundant parens
(which is against coding conventions).

>> Looks good otherwise, thanks.  We just need to sort out the build
>> problem.
>
> I added to that bug you mentioned (in another mail), as I determined
> that the problem flag is -foptimize-sibling-calls combined with -O1.
> With -O0, it will run just fine, so there's another one or more flags
> enabled by -O1 that are causing the fluke.  No idea if it's
> genautomata itself, since I peeked into SVN and that source file
> hasn't seen any activity in two months.  I figure it must be one of
> the other files that get linked in.

Thanks.  Haven't had time to look at the PR yet.  Will be the weekend
at the earliest now.

> Also, what about a test case?  This is pretty dangerous on Rev 2.5
> R10Ks, potentially locking them up, so I don't know if a testcase
> would be necessary.

A testcase would be nice, yes.  It helps people who are testing on
non-R10K hardware.  It doesn't need to be an execution test though:
just write a scan-assembler test to make sure that all __sync_*()
builtins use branch-likely instructions.  See other gcc.target/mips
tests for inspiration.

> +  /* Make sure that branch-likely instructions available when using
> +     -mfix-r10000.  The instructions are not available if either:

Minor nit, but I think the comment is easier to read if we keep the
suggested blank line here.

> +	1. -mno-branch-likely was passed.
> +	2. The selected ISA does not support branch-likely and
> +	   the command line does not include -mbranch-likely  */

You lost the "." at the end of the comment (coding conventions require it).

> diff -Naurp -x .svn gcc.orig/gcc/doc/invoke.texi gcc/gcc/doc/invoke.texi
> --- gcc.orig/gcc/doc/invoke.texi	2008-10-30 22:14:29.000000000 -0400
> +++ gcc/gcc/doc/invoke.texi	2008-11-11 22:38:37.000000000 -0500
> @@ -666,7 +666,7 @@ Objective-C and Objective-C++ Dialects}.
>  -mdivide-traps  -mdivide-breaks @gol
>  -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
>  -mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
> --mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
> +-mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 -mfix-r10000 -mno-fix-r10000 @gol
>  -mfix-vr4120  -mno-fix-vr4120  -mfix-vr4130  -mno-fix-vr4130 @gol
>  -mfix-sb1  -mno-fix-sb1 @gol
>  -mflush-func=@var{func}  -mno-flush-func @gol

This is preformatted text, so the new line is too long.  Push it onto the
next, and push the -m{no-,}-fix-vr4130 options down to the following line.

You need to document what -mfix-r10000 does as well. ;)  See the other
-mfix-* options for the general idea.

Richard

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

[-- Attachment #1: Type: text/plain, Size: 3418 bytes --]

Richard Sandiford wrote:
> 
> Well, I suppose the current sorry() is too terse even for an
> explicit -mfix-r10000.  How about:
> 
>   sorry ("%qs requires branch-likely instructions\n", "-mfix-r10000");
> 
> (Done that way so that we can reuse any translations in cases where
> we need branch-likely instructions for some other option.)  I think
> that's OK even for "-march=r10000 -mno-branch-likely"; the documentation
> should make it clear that -mfix-r10000 is the default for -march=r10000.

Done.


>> Perhaps:
>>
>> if ((mips_matching_cpu_name_p (mips_arch_info->name, "r12000") ||
>>       mips_matching_cpu_name_p (mips_arch_info->name, "r14000") ||
>>       mips_matching_cpu_name_p (mips_arch_info->name, "r16000"))
>>      && TARGET_FIX_R10000)
>> {
>>    sorry ("R10000 Errata fix not necessary on R12000 and greater CPUs");
>> }
> 
> No, there's no need for anything like this.

Gotcha.  I guess any such combination, if used, will have to be dealt with by 
the CPU.  All the binaries on my Octane, running an R14000 are -march=r10000 
anyways, so when 4.4 final comes out and I start rebuilding stuff, I'll notice 
pretty fast if something is amiss.


> 'Fraid so. ;)  You had those right the first time.

Fixed!


> Thanks.  Haven't had time to look at the PR yet.  Will be the weekend
> at the earliest now.

Yeah, it's an odd one.  It forced me to use the early xgcc compiler to test my 
testcases, since I can't fully build the compiler just yet until this PR gets 
addressed.


> A testcase would be nice, yes.  It helps people who are testing on
> non-R10K hardware.  It doesn't need to be an execution test though:
> just write a scan-assembler test to make sure that all __sync_*()
> builtins use branch-likely instructions.  See other gcc.target/mips
> tests for inspiration.

Okay, I think I got these right.  There's 16 of them, fix-r10000-1.c through 
fix-r10000-16.c.  Each file tests a single __sync_* function, and has the 
scan-assembler look for the 'beql' instruction in the output.  I test compiled 
all of them by hand and checked their output with both -mno-fix-r10000 and 
-mfix-r10000, and a diff shows that exact instruction changing, so I think these 
all pass.

I couldn't tell which __sync_* function outputs the asm code found in 
MIPS_SYNC_NEW_OP and MIPS_SYNC_NEW_NAND, but other than that, the output all 
looked good.

I may not need fix-r10000-15.c, since that calls __sync_synchronize(), which 
doesn't output any branch-likely instructions at all.  Wasn't sure where it's 
proper to call that function from (or if it even needs testing).


> Minor nit, but I think the comment is easier to read if we keep the
> suggested blank line here.
> 
[snip]
> You lost the "." at the end of the comment (coding conventions require it).

Fixed.


> This is preformatted text, so the new line is too long.  Push it onto the
> next, and push the -m{no-,}-fix-vr4130 options down to the following line.
> 
> You need to document what -mfix-r10000 does as well. ;)  See the other
> -mfix-* options for the general idea.

Fixed, and added the documentation.  If this looks good, then I'll cut a final 
w/ the changelog notations.

Thanks!

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

[-- Attachment #2: gcc-4.4-trunk-fixr10k-z4.patch --]
[-- Type: text/plain, Size: 24637 bytes --]

diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.c gcc/gcc/config/mips/mips.c
--- gcc.orig/gcc/config/mips/mips.c	2008-11-06 00:35:19.000000000 -0500
+++ gcc/gcc/config/mips/mips.c	2008-11-13 20:42:45.000000000 -0500
@@ -6909,6 +6909,7 @@ mips_print_operand_reloc (FILE *file, rt
    '#'	Print a nop if in a ".set noreorder" block.
    '/'	Like '#', but do nothing within a delayed-branch sequence.
    '?'	Print "l" if mips_branch_likely is true
+   '~'	Print a nop if mips_branch_likely is true
    '.'	Print the name of the register with a hard-wired zero (zero or $0).
    '@'	Print the name of the assembler temporary register (at or $1).
    '^'	Print the name of the pic call-through register (t9 or $25).
@@ -6983,6 +6984,11 @@ mips_print_operand_punctuation (FILE *fi
 	putc ('l', file);
       break;
 
+    case '~':
+      if (mips_branch_likely)
+	fputs ("\n\tnop", file);
+      break;
+
     case '.':
       fputs (reg_names[GP_REG_FIRST + 0], file);
       break;
@@ -7026,7 +7032,7 @@ mips_init_print_operand_punct (void)
 {
   const char *p;
 
-  for (p = "()[]<>*#/?.@^+$|-"; *p; p++)
+  for (p = "()[]<>*#/?~.@^+$|-"; *p; p++)
     mips_print_operand_punct[(unsigned char) *p] = true;
 }
 
@@ -10250,6 +10256,17 @@ mips_output_order_conditional_branch (rt
   return mips_output_conditional_branch (insn, operands, branch[1], branch[0]);
 }
 \f
+/* Return a template for the __sync_* functions after setting mips_branch_likely
+   to the value of TARGET_FIX_R10000 to enable a proper workaround of R10000
+   errata.  */
+
+const char *
+mips_output_sync_insn (const char *template)
+{
+  mips_branch_likely = TARGET_FIX_R10000;
+  return template;
+}
+\f
 /* Return the assembly code for DIV or DDIV instruction DIVISION, which has
    the operands given by OPERANDS.  Add in a divide-by-zero check if needed.
 
@@ -13971,6 +13988,24 @@ mips_override_options (void)
       && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
     target_flags |= MASK_FIX_R4400;
 
+  /* Default to working around R10000 errata only if the processor
+     was selected explicitly.  */
+  if ((target_flags_explicit & MASK_FIX_R10000) == 0
+      && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
+    target_flags |= MASK_FIX_R10000;
+
+  /* Make sure that branch-likely instructions available when using
+     -mfix-r10000.  The instructions are not available if either:
+
+	1. -mno-branch-likely was passed.
+	2. The selected ISA does not support branch-likely and
+	   the command line does not include -mbranch-likely.  */
+  if (TARGET_FIX_R10000
+      && ((target_flags_explicit & MASK_BRANCHLIKELY) == 0
+          ? !ISA_HAS_BRANCHLIKELY
+          : !TARGET_BRANCHLIKEL))
+    sorry ("%qs requires branch-likely instructions\n", "-mfix-r10000");
+
   /* Save base state of options.  */
   mips_base_target_flags = target_flags;
   mips_base_delayed_branch = flag_delayed_branch;
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.h gcc/gcc/config/mips/mips.h
--- gcc.orig/gcc/config/mips/mips.h	2008-11-11 22:33:57.000000000 -0500
+++ gcc/gcc/config/mips/mips.h	2008-11-11 22:38:37.000000000 -0500
@@ -3089,7 +3089,7 @@ while (0)
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3114,7 +3114,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3134,7 +3134,7 @@ while (0)
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3159,7 +3159,7 @@ while (0)
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3192,7 +3192,7 @@ while (0)
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3222,7 +3222,7 @@ while (0)
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3242,7 +3242,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3259,7 +3259,7 @@ while (0)
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3276,7 +3276,7 @@ while (0)
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3295,7 +3295,7 @@ while (0)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3314,7 +3314,7 @@ while (0)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3332,7 +3332,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%-%]%>%)"
 
@@ -3356,7 +3356,7 @@ while (0)
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
diff -Naurp -x .svn gcc.orig/gcc/config/mips/mips.opt gcc/gcc/config/mips/mips.opt
--- gcc.orig/gcc/config/mips/mips.opt	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/mips.opt	2008-11-11 22:38:37.000000000 -0500
@@ -112,6 +112,10 @@ mfix-r4400
 Target Report Mask(FIX_R4400)
 Work around certain R4400 errata
 
+mfix-r10000
+Target Report Mask(FIX_R10000)
+Work around certain R10000 errata
+
 mfix-sb1
 Target Report Var(TARGET_FIX_SB1)
 Work around errata for early SB-1 revision 2 cores
diff -Naurp -x .svn gcc.orig/gcc/config/mips/sync.md gcc/gcc/config/mips/sync.md
--- gcc.orig/gcc/config/mips/sync.md	2008-10-30 22:20:27.000000000 -0400
+++ gcc/gcc/config/mips/sync.md	2008-11-13 20:44:20.000000000 -0500
@@ -43,9 +43,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP ("<d>", "li");
+    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP ("<d>", "li"));
   else
-    return MIPS_COMPARE_AND_SWAP ("<d>", "move");
+    return mips_output_sync_insn (MIPS_COMPARE_AND_SWAP ("<d>", "move"));
 }
   [(set_attr "length" "32")])
 
@@ -76,9 +76,11 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP)));
   else
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP)));
 }
   [(set_attr "length" "40,36")])
 
@@ -91,9 +93,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_OP ("<d>", "<d>addu");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -124,7 +126,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP)));
 }
   [(set_attr "length" "40")])
 
@@ -160,8 +163,9 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
-				MIPS_SYNC_OLD_OP_12_NOT_NOP_REG);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
+	     MIPS_SYNC_OLD_OP_12_NOT_NOP_REG)));
 }
   [(set_attr "length" "40")])
 
@@ -202,7 +206,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP)));
 }
   [(set_attr "length" "40")])
 
@@ -233,7 +238,8 @@
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT)));
 }
   [(set_attr "length" "44")])
 
@@ -267,8 +273,9 @@
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
-				MIPS_SYNC_OLD_OP_12_NOT_NOT_REG);	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
+	     MIPS_SYNC_OLD_OP_12_NOT_NOT_REG)));
 }
   [(set_attr "length" "44")])
 
@@ -307,7 +314,8 @@
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT)));
 }
   [(set_attr "length" "40")])
 
@@ -319,7 +327,7 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OP ("<d>", "<d>subu");	
+  return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<d>subu"));
 }
   [(set_attr "length" "28")])
 
@@ -334,9 +342,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addu");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -350,7 +358,7 @@
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OLD_OP ("<d>", "<d>subu");	
+  return mips_output_sync_insn (MIPS_SYNC_OLD_OP ("<d>", "<d>subu"));
 }
   [(set_attr "length" "28")])
 
@@ -365,9 +373,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addu");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -381,7 +389,7 @@
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_NEW_OP ("<d>", "<d>subu");	
+  return mips_output_sync_insn (MIPS_SYNC_NEW_OP ("<d>", "<d>subu"));
 }
   [(set_attr "length" "28")])
 
@@ -394,9 +402,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<immediate_insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<immediate_insn>"));
   else
-    return MIPS_SYNC_OP ("<d>", "<insn>");	
+    return mips_output_sync_insn (MIPS_SYNC_OP ("<d>", "<insn>"));
 }
   [(set_attr "length" "28")])
 
@@ -411,9 +419,11 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_OLD_OP ("<d>", "<insn>")));
 }
   [(set_attr "length" "28")])
 
@@ -428,9 +438,11 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<insn>");	
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_NEW_OP ("<d>", "<insn>")));
 }
   [(set_attr "length" "28")])
 
@@ -441,9 +453,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NAND ("<d>", "andi");	
+    return mips_output_sync_insn (MIPS_SYNC_NAND ("<d>", "andi"));
   else
-    return MIPS_SYNC_NAND ("<d>", "and");	
+    return mips_output_sync_insn (MIPS_SYNC_NAND ("<d>", "and"));
 }
   [(set_attr "length" "32")])
 
@@ -456,9 +468,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_NAND ("<d>", "andi");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_NAND ("<d>", "andi"));
   else
-    return MIPS_SYNC_OLD_NAND ("<d>", "and");	
+    return mips_output_sync_insn (MIPS_SYNC_OLD_NAND ("<d>", "and"));
 }
   [(set_attr "length" "32")])
 
@@ -471,9 +483,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_NAND ("<d>", "andi");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_NAND ("<d>", "andi"));
   else
-    return MIPS_SYNC_NEW_NAND ("<d>", "and");	
+    return mips_output_sync_insn (MIPS_SYNC_NEW_NAND ("<d>", "and"));
 }
   [(set_attr "length" "32")])
 
@@ -486,9 +498,9 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE ("<d>", "li");
+    return mips_output_sync_insn (MIPS_SYNC_EXCHANGE ("<d>", "li"));
   else
-    return MIPS_SYNC_EXCHANGE ("<d>", "move");
+    return mips_output_sync_insn (MIPS_SYNC_EXCHANGE ("<d>", "move"));
 }
   [(set_attr "length" "24")])
 
@@ -516,8 +528,10 @@
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP)));
   else
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP);
+    return (mips_output_sync_insn
+	    (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP)));
 }
   [(set_attr "length" "28,24")])
diff -Naurp -x .svn gcc.orig/gcc/doc/invoke.texi gcc/gcc/doc/invoke.texi
--- gcc.orig/gcc/doc/invoke.texi	2008-10-30 22:14:29.000000000 -0400
+++ gcc/gcc/doc/invoke.texi	2008-11-14 02:20:06.000000000 -0500
@@ -666,8 +666,9 @@ Objective-C and Objective-C++ Dialects}.
 -mdivide-traps  -mdivide-breaks @gol
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
 -mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
--mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
--mfix-vr4120  -mno-fix-vr4120  -mfix-vr4130  -mno-fix-vr4130 @gol
+-mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400
+-mfix-r10000 -mno-fix-r10000 @gol
+-mfix-vr4120  -mno-fix-vr4120 -mfix-vr4130  -mno-fix-vr4130 @gol
 -mfix-sb1  -mno-fix-sb1 @gol
 -mflush-func=@var{func}  -mno-flush-func @gol
 -mbranch-cost=@var{num}  -mbranch-likely  -mno-branch-likely @gol
@@ -12827,6 +12828,20 @@ A double-word or a variable shift may gi
 immediately after starting an integer division.
 @end itemize
 
+@item -mfix-r10000
+@itemx -mno-fix-r10000
+@opindex mfix-r10000
+@opindex mno-fix-r10000
+LL/SC workaround for R10000 errata in specific revisions of the CPU
+silicon.  Revs before 3.0 will misbehave on atomic operations, and revs
+2.6 and below will deadlock due to other conflicting errata.
+
+This workaround only works for MIPS-II and above binaries due to the
+use of branch-likely instructions (@code{beql}, @code{beqzl}).  Although
+a workaround exists for MIPS-I, it is not implemented by this flag.
+
+This flag is enabled by default when @code{-march=r10000} is used.
+
 @item -mfix-vr4120
 @itemx -mno-fix-vr4120
 @opindex mfix-vr4120
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-1.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-1.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-1.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-1.c	2008-11-14 02:31:54.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_fetch_and_add (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-10.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-10.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-10.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-10.c	2008-11-14 02:34:02.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_and_and_fetch (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-11.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-11.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-11.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-11.c	2008-11-14 02:34:07.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_xor_and_fetch (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-12.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-12.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-12.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-12.c	2008-11-14 02:34:12.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_nand_and_fetch (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-13.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-13.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-13.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-13.c	2008-11-14 02:34:18.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_bool_compare_and_swap (&z, 0, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-14.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-14.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-14.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-14.c	2008-11-14 02:34:24.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_val_compare_and_swap (&z, 0, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-15.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-15.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-15.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-15.c	2008-11-14 02:34:31.000000000 -0500
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  __sync_synchronize ();
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-16.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-16.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-16.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-16.c	2008-11-14 02:34:37.000000000 -0500
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_lock_test_and_set (&z, 42);
+  __sync_lock_release (&z);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-2.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-2.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-2.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-2.c	2008-11-14 02:33:10.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_fetch_and_sub (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-3.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-3.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-3.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-3.c	2008-11-14 02:33:17.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_fetch_and_or (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-4.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-4.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-4.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-4.c	2008-11-14 02:33:23.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_fetch_and_and (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-5.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-5.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-5.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-5.c	2008-11-14 02:33:30.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_fetch_and_xor (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-6.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-6.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-6.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-6.c	2008-11-14 02:33:37.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_fetch_and_nand (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-7.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-7.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-7.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-7.c	2008-11-14 02:33:42.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_add_and_fetch (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-8.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-8.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-8.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-8.c	2008-11-14 02:33:49.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_sub_and_fetch (&z, 42);
+}
diff -Naurp -x .svn gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-9.c gcc/gcc/testsuite/gcc.target/mips/fix-r10000-9.c
--- gcc.orig/gcc/testsuite/gcc.target/mips/fix-r10000-9.c	1969-12-31 19:00:00.000000000 -0500
+++ gcc/gcc/testsuite/gcc.target/mips/fix-r10000-9.c	2008-11-14 02:33:55.000000000 -0500
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler "beql" } } */
+
+NOMIPS16 void
+foo(void)
+{
+  unsigned z = 0;
+
+  __sync_or_and_fetch (&z, 42);
+}

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

Hi,

Thanks for the patch.

Kumba <kumba@gentoo.org> writes:
>> A testcase would be nice, yes.  It helps people who are testing on
>> non-R10K hardware.  It doesn't need to be an execution test though:
>> just write a scan-assembler test to make sure that all __sync_*()
>> builtins use branch-likely instructions.  See other gcc.target/mips
>> tests for inspiration.
>
> Okay, I think I got these right.  There's 16 of them, fix-r10000-1.c
> through fix-r10000-16.c.  Each file tests a single __sync_* function,
> and has the scan-assembler look for the 'beql' instruction in the
> output.  I test compiled all of them by hand and checked their output
> with both -mno-fix-r10000 and -mfix-r10000, and a diff shows that
> exact instruction changing, so I think these all pass.
>
> I couldn't tell which __sync_* function outputs the asm code found in 
> MIPS_SYNC_NEW_OP and MIPS_SYNC_NEW_NAND, but other than that, the output all 
> looked good.

__sync_<op>_and_fetch uses MIPS_SYNC_NEW_OP while __sync_fetch_and_<op>
uses MIPS_SYNC_OLD_OP.  So your tests did cover the necessary functions.
The problem was that the tests ignored the return value, so
__sync_<op>_and_fetch reduced to __sync_fetch_and_<op>.

> I may not need fix-r10000-15.c, since that calls __sync_synchronize(), which 
> doesn't output any branch-likely instructions at all.  Wasn't sure where it's 
> proper to call that function from (or if it even needs testing).

Yeah, that test is wrong.  __sync_synchronize isn't a loop (and isn't
meant to be a loop) so the test rightly fails.

> Fixed, and added the documentation.  If this looks good, then I'll cut
> a final w/ the changelog notations.

Yeah, it looks generally good.  I think we've got to the point where
it's easier for me to make changes directly rather than ask you to
follow a tortuous list of vaguely-described requests, so:

  - I added a missing @gol after "-mfix-r4400".

  - I tweaked the documentation so that it was more consistent with the
    other -mfix-* options.  Let me know if you spot a problem with the
    new version, or if you aren't happy with it.

  - I changed the name of the helper function from mips_output_sync_insn
    (my original suggestion) to mips_output_sync_loop, which seems a
    bit more descriptive.  Sorry for going back on myself.  Related...

  - ...I changed the name of the parameter from TEMPLATE to LOOP to
    avoid a bootstrap-breaking warning about using a C++ identifier.
    (Again my fault.  I'd used TEMPLATE when suggesting the function,
    but it was a completely untested suggestion.)

  - I added a prototype to mips-protos.h, again to avoid a bootstrap-
    breaking warning.

  - I removed the "\n" from the sorry message (my fault again).

  - I fixed a typo: s/!TARGET_BRANCHLIKEL/!TARGET_BRANCHLIKELY/.
    GCC wouldn't build without this, so perhaps the posted patch
    wasn't the final one.

  - I made the tests check for "\tbeql\t", which is a bit more
    robust than plain "beql".  (OK, it's not likely to make a
    difference for this particular example, but it seems like
    good practice.)

  - I changed the tests to operate on caller-provided memory.
    The old versions operated on automatic variables that obviously
    couldn't escape, sp I was worried they might be optimised in future.

  - I made the subtraction tests take the subtrahend as an argument,
    because subtracting 42 reduces to adding -42.

  - I added "short" and "char" versions of each test function.

  - I made the test functions return the result of the __sync_* builtin,
    for the reasons given above.

  - As discussed above, I removed the original test 15 and
    renamed 16 to 15.

  - I tweaked the formatting in a couple of places, but nothing major.

Applied with those changes.  I've attached the final changelog and
patch below.  Thanks for the contribution, and for your patience.

Richard


gcc/
2008-11-15  Joshua Kinard  <kumba@gentoo.org>

	* doc/invoke.texi (-mfix-r10000): Document.
	* config/mips/mips.opt (mfix-r10000): New option.
	* config/mips/mips-protos.h (mips_output_sync_loop): Declare.
	* config/mips/mips.h (MIPS_COMPARE_AND_SWAP): Use %?.
	(MIPS_COMPARE_AND_SWAP_12): Likewise.
	(MIPS_SYNC_OP): Likewise.
	(MIPS_SYNC_OP_12): Likewise.
	(MIPS_SYNC_OLD_OP_12): Likewise.
	(MIPS_SYNC_NEW_OP_12): Likewise.
	(MIPS_SYNC_OLD_OP): Likewise.
	(MIPS_SYNC_NAND): Likewise.
	(MIPS_SYNC_OLD_NAND): Likewise.
	(MIPS_SYNC_EXCHANGE): Likewise.
	(MIPS_SYNC_EXCHANGE_12): Likewise.
	(MIPS_SYNC_NEW_OP): Likewise, using %~ to fill branch-likely
	delay slots.
	(MIPS_SYNC_NEW_NAND): Likewise.
	* config/mips/mips.c (mips_print_operand_punctuation): Handle '~'.
	(mips_init_print_operand_punct): Treat '~' as a punctuation character.
	(mips_output_sync_loop): New function.
	(mips_override_options): Make -march=r10000 imply -mfix-r10000.
	Make -mfix-r10000 require branch-likely instructions.
	* config/mips/sync.md (sync_compare_and_swap<mode>): Use
	mips_output_sync_loop.
	(compare_and_swap_12): Likewise.
	(sync_add<mode>): Likewise.
	(sync_<optab>_12): Likewise.
	(sync_old_<optab>_12): Likewise.
	(sync_new_<optab>_12): Likewise.
	(sync_nand_12): Likewise.
	(sync_old_nand_12): Likewise.
	(sync_new_nand_12): Likewise.
	(sync_sub<mode>): Likewise.
	(sync_old_add<mode>): Likewise.
	(sync_old_sub<mode>): Likewise.
	(sync_new_add<mode>): Likewise.
	(sync_new_sub<mode>): Likewise.
	(sync_<optab><mode>): Likewise.
	(sync_old_<optab><mode>): Likewise.
	(sync_new_<optab><mode>): Likewise.
	(sync_nand<mode>): Likewise.
	(sync_old_nand<mode>): Likewise.
	(sync_new_nand<mode>): Likewise.
	(sync_lock_test_and_set<mode>): Likewise.
	(test_and_set_12): Likewise.

gcc/testsuite/
2008-11-15  Joshua Kinard  <kumba@gentoo.org>
	    Richard Sandiford  <rdsandiford@goolemail.com>

	* gcc.target/mips/fix-r10000-1.c: New test.
	* gcc.target/mips/fix-r10000-2.c: Likewise.
	* gcc.target/mips/fix-r10000-3.c: Likewise.
	* gcc.target/mips/fix-r10000-4.c: Likewise.
	* gcc.target/mips/fix-r10000-5.c: Likewise.
	* gcc.target/mips/fix-r10000-6.c: Likewise.
	* gcc.target/mips/fix-r10000-7.c: Likewise.
	* gcc.target/mips/fix-r10000-8.c: Likewise.
	* gcc.target/mips/fix-r10000-9.c: Likewise.
	* gcc.target/mips/fix-r10000-10.c: Likewise.
	* gcc.target/mips/fix-r10000-11.c: Likewise.
	* gcc.target/mips/fix-r10000-12.c: Likewise.
	* gcc.target/mips/fix-r10000-13.c: Likewise.
	* gcc.target/mips/fix-r10000-14.c: Likewise.
	* gcc.target/mips/fix-r10000-15.c: Likewise.

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	2008-11-15 14:19:02.000000000 +0000
+++ gcc/doc/invoke.texi	2008-11-15 14:21:38.000000000 +0000
@@ -668,8 +668,8 @@ Objective-C and Objective-C++ Dialects}.
 -mmemcpy  -mno-memcpy  -mlong-calls  -mno-long-calls @gol
 -mmad  -mno-mad  -mfused-madd  -mno-fused-madd  -nocpp @gol
 -mfix-r4000  -mno-fix-r4000  -mfix-r4400  -mno-fix-r4400 @gol
--mfix-vr4120  -mno-fix-vr4120  -mfix-vr4130  -mno-fix-vr4130 @gol
--mfix-sb1  -mno-fix-sb1 @gol
+-mfix-r10000 -mno-fix-r10000  -mfix-vr4120  -mno-fix-vr4120 @gol
+-mfix-vr4130  -mno-fix-vr4130  -mfix-sb1  -mno-fix-sb1 @gol
 -mflush-func=@var{func}  -mno-flush-func @gol
 -mbranch-cost=@var{num}  -mbranch-likely  -mno-branch-likely @gol
 -mfp-exceptions -mno-fp-exceptions @gol
@@ -12833,6 +12833,22 @@ A double-word or a variable shift may gi
 immediately after starting an integer division.
 @end itemize
 
+@item -mfix-r10000
+@itemx -mno-fix-r10000
+@opindex mfix-r10000
+@opindex mno-fix-r10000
+Work around certain R10000 errata:
+@itemize @minus
+@item
+@code{ll}/@code{sc} sequences may not behave atomically on revisions
+prior to 3.0.  They may deadlock on revisions 2.6 and earlier.
+@end itemize
+
+This option can only be used if the target architecture supports
+branch-likely instructions.  @option{-mfix-r10000} is the default when
+@option{-march=r10000} is used; @option{-mno-fix-r10000} is the default
+otherwise.
+
 @item -mfix-vr4120
 @itemx -mno-fix-vr4120
 @opindex mfix-vr4120
Index: gcc/config/mips/mips.opt
===================================================================
--- gcc/config/mips/mips.opt	2008-11-15 14:19:02.000000000 +0000
+++ gcc/config/mips/mips.opt	2008-11-15 14:21:08.000000000 +0000
@@ -112,6 +112,10 @@ mfix-r4400
 Target Report Mask(FIX_R4400)
 Work around certain R4400 errata
 
+mfix-r10000
+Target Report Mask(FIX_R10000)
+Work around certain R10000 errata
+
 mfix-sb1
 Target Report Var(TARGET_FIX_SB1)
 Work around errata for early SB-1 revision 2 cores
Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc/config/mips/mips-protos.h	2008-11-15 14:19:02.000000000 +0000
+++ gcc/config/mips/mips-protos.h	2008-11-15 14:21:08.000000000 +0000
@@ -300,6 +300,7 @@ extern const char *mips_output_load_labe
 extern const char *mips_output_conditional_branch (rtx, rtx *, const char *,
 						   const char *);
 extern const char *mips_output_order_conditional_branch (rtx, rtx *, bool);
+extern const char *mips_output_sync_loop (const char *);
 extern const char *mips_output_division (const char *, rtx *);
 extern unsigned int mips_hard_regno_nregs (int, enum machine_mode);
 extern bool mips_linked_madd_p (rtx, rtx);
Index: gcc/config/mips/mips.h
===================================================================
--- gcc/config/mips/mips.h	2008-11-15 14:19:02.000000000 +0000
+++ gcc/config/mips/mips.h	2008-11-15 14:21:08.000000000 +0000
@@ -3090,7 +3090,7 @@ #define MIPS_COMPARE_AND_SWAP(SUFFIX, OP
   "\tbne\t%0,%z2,2f\n"				\
   "\t" OP "\t%@,%3\n"				\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3115,7 +3115,7 @@ #define MIPS_COMPARE_AND_SWAP_12(OPS)		\
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)\n"				\
   "2:\n"
@@ -3135,7 +3135,7 @@ #define MIPS_SYNC_OP(SUFFIX, INSN)		\
   "1:\tll" SUFFIX "\t%@,%0\n"			\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3160,7 +3160,7 @@ #define MIPS_SYNC_OP_12(INSN, NOT_OP)		\
   "\tand\t%4,%4,%1\n"				\
   "\tor\t%@,%@,%4\n"				\
   "\tsc\t%@,%0\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3193,7 +3193,7 @@ #define MIPS_SYNC_OLD_OP_12(INSN, NOT_OP
   "\tand\t%5,%5,%2\n"				\
   "\tor\t%@,%@,%5\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3223,7 +3223,7 @@ #define MIPS_SYNC_NEW_OP_12(INSN, NOT_OP
   "\tand\t%0,%0,%2\n"				\
   "\tor\t%@,%@,%0\n"				\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3243,7 +3243,7 @@ #define MIPS_SYNC_OLD_OP(SUFFIX, INSN)		
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3260,7 +3260,7 @@ #define MIPS_SYNC_NEW_OP(SUFFIX, INSN)		
   "1:\tll" SUFFIX "\t%0,%1\n"			\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3277,7 +3277,7 @@ #define MIPS_SYNC_NAND(SUFFIX, INSN)		\
   "\tnor\t%@,%@,%.\n"				\
   "\t" INSN "\t%@,%@,%1\n"			\
   "\tsc" SUFFIX "\t%@,%0\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3296,7 +3296,7 @@ #define MIPS_SYNC_OLD_NAND(SUFFIX, INSN)
   "\tnor\t%@,%0,%.\n"				\
   "\t" INSN "\t%@,%@,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
@@ -3315,7 +3315,7 @@ #define MIPS_SYNC_NEW_NAND(SUFFIX, INSN)
   "\tnor\t%0,%0,%.\n"				\
   "\t" INSN "\t%@,%0,%2\n"			\
   "\tsc" SUFFIX "\t%@,%1\n"			\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b%~\n"			\
   "\t" INSN "\t%0,%0,%2\n"			\
   "\tsync%-%]%>%)"
 
@@ -3333,7 +3333,7 @@ #define MIPS_SYNC_EXCHANGE(SUFFIX, OP)		
   "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%-%]%>%)"
 
@@ -3357,7 +3357,7 @@ #define MIPS_SYNC_EXCHANGE_12(OPS)      
   "\tand\t%@,%0,%3\n"				\
   OPS						\
   "\tsc\t%@,%1\n"				\
-  "\tbeq\t%@,%.,1b\n"				\
+  "\tbeq%?\t%@,%.,1b\n"				\
   "\tnop\n"					\
   "\tsync%-%]%>%)"
 
Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2008-11-15 14:19:02.000000000 +0000
+++ gcc/config/mips/mips.c	2008-11-15 14:21:08.000000000 +0000
@@ -6909,6 +6909,7 @@ mips_print_operand_reloc (FILE *file, rt
    '#'	Print a nop if in a ".set noreorder" block.
    '/'	Like '#', but do nothing within a delayed-branch sequence.
    '?'	Print "l" if mips_branch_likely is true
+   '~'	Print a nop if mips_branch_likely is true
    '.'	Print the name of the register with a hard-wired zero (zero or $0).
    '@'	Print the name of the assembler temporary register (at or $1).
    '^'	Print the name of the pic call-through register (t9 or $25).
@@ -6983,6 +6984,11 @@ mips_print_operand_punctuation (FILE *fi
 	putc ('l', file);
       break;
 
+    case '~':
+      if (mips_branch_likely)
+	fputs ("\n\tnop", file);
+      break;
+
     case '.':
       fputs (reg_names[GP_REG_FIRST + 0], file);
       break;
@@ -7026,7 +7032,7 @@ mips_init_print_operand_punct (void)
 {
   const char *p;
 
-  for (p = "()[]<>*#/?.@^+$|-"; *p; p++)
+  for (p = "()[]<>*#/?~.@^+$|-"; *p; p++)
     mips_print_operand_punct[(unsigned char) *p] = true;
 }
 
@@ -10250,6 +10256,17 @@ mips_output_order_conditional_branch (rt
   return mips_output_conditional_branch (insn, operands, branch[1], branch[0]);
 }
 \f
+/* Return the assembly code for __sync_*() loop LOOP.  The loop should support
+   both normal and likely branches, using %? and %~ where appropriate.  */
+
+const char *
+mips_output_sync_loop (const char *loop)
+{
+  /* Use branch-likely instructions to work around the LL/SC R10000 errata.  */
+  mips_branch_likely = TARGET_FIX_R10000;
+  return loop;
+}
+\f
 /* Return the assembly code for DIV or DDIV instruction DIVISION, which has
    the operands given by OPERANDS.  Add in a divide-by-zero check if needed.
 
@@ -13971,6 +13988,24 @@ mips_override_options (void)
       && mips_matching_cpu_name_p (mips_arch_info->name, "r4400"))
     target_flags |= MASK_FIX_R4400;
 
+  /* Default to working around R10000 errata only if the processor
+     was selected explicitly.  */
+  if ((target_flags_explicit & MASK_FIX_R10000) == 0
+      && mips_matching_cpu_name_p (mips_arch_info->name, "r10000"))
+    target_flags |= MASK_FIX_R10000;
+
+  /* Make sure that branch-likely instructions available when using
+     -mfix-r10000.  The instructions are not available if either:
+
+	1. -mno-branch-likely was passed.
+	2. The selected ISA does not support branch-likely and
+	   the command line does not include -mbranch-likely.  */
+  if (TARGET_FIX_R10000
+      && ((target_flags_explicit & MASK_BRANCHLIKELY) == 0
+          ? !ISA_HAS_BRANCHLIKELY
+          : !TARGET_BRANCHLIKELY))
+    sorry ("%qs requires branch-likely instructions", "-mfix-r10000");
+
   /* Save base state of options.  */
   mips_base_target_flags = target_flags;
   mips_base_delayed_branch = flag_delayed_branch;
Index: gcc/config/mips/sync.md
===================================================================
--- gcc/config/mips/sync.md	2008-11-15 14:19:02.000000000 +0000
+++ gcc/config/mips/sync.md	2008-11-15 14:21:08.000000000 +0000
@@ -43,9 +43,9 @@ (define_insn "sync_compare_and_swap<mode
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP ("<d>", "li");
+    return mips_output_sync_loop (MIPS_COMPARE_AND_SWAP ("<d>", "li"));
   else
-    return MIPS_COMPARE_AND_SWAP ("<d>", "move");
+    return mips_output_sync_loop (MIPS_COMPARE_AND_SWAP ("<d>", "move"));
 }
   [(set_attr "length" "32")])
 
@@ -76,9 +76,11 @@ (define_insn "compare_and_swap_12"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP);
+    return (mips_output_sync_loop
+	    (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_NONZERO_OP)));
   else
-    return MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP);
+    return (mips_output_sync_loop
+	    (MIPS_COMPARE_AND_SWAP_12 (MIPS_COMPARE_AND_SWAP_12_ZERO_OP)));
 }
   [(set_attr "length" "40,36")])
 
@@ -91,9 +93,9 @@ (define_insn "sync_add<mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_loop (MIPS_SYNC_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_OP ("<d>", "<d>addu");	
+    return mips_output_sync_loop (MIPS_SYNC_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -124,7 +126,8 @@ (define_insn "sync_<optab>_12"
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP);	
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_OP_12 ("<insn>", MIPS_SYNC_OP_12_NOT_NOP)));
 }
   [(set_attr "length" "40")])
 
@@ -160,8 +163,9 @@ (define_insn "sync_old_<optab>_12"
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
-				MIPS_SYNC_OLD_OP_12_NOT_NOP_REG);	
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_OLD_OP_12 ("<insn>", MIPS_SYNC_OLD_OP_12_NOT_NOP,
+	    			  MIPS_SYNC_OLD_OP_12_NOT_NOP_REG)));
 }
   [(set_attr "length" "40")])
 
@@ -202,7 +206,8 @@ (define_insn "sync_new_<optab>_12"
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP);
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_NEW_OP_12 ("<insn>", MIPS_SYNC_NEW_OP_12_NOT_NOP)));
 }
   [(set_attr "length" "40")])
 
@@ -233,7 +238,8 @@ (define_insn "sync_nand_12"
    (clobber (match_scratch:SI 4 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT);	
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_OP_12 ("and", MIPS_SYNC_OP_12_NOT_NOT)));
 }
   [(set_attr "length" "44")])
 
@@ -267,8 +273,9 @@ (define_insn "sync_old_nand_12"
    (clobber (match_scratch:SI 5 "=&d"))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
-				MIPS_SYNC_OLD_OP_12_NOT_NOT_REG);	
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_OLD_OP_12 ("and", MIPS_SYNC_OLD_OP_12_NOT_NOT,
+				  MIPS_SYNC_OLD_OP_12_NOT_NOT_REG)));
 }
   [(set_attr "length" "44")])
 
@@ -307,7 +314,8 @@ (define_insn "sync_new_nand_12"
 	   (match_dup 4)] UNSPEC_SYNC_NEW_OP_12))]
   "GENERATE_LL_SC"
 {
-    return MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT);
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_NEW_OP_12 ("and", MIPS_SYNC_NEW_OP_12_NOT_NOT)));
 }
   [(set_attr "length" "40")])
 
@@ -319,7 +327,7 @@ (define_insn "sync_sub<mode>"
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OP ("<d>", "<d>subu");	
+  return mips_output_sync_loop (MIPS_SYNC_OP ("<d>", "<d>subu"));
 }
   [(set_attr "length" "28")])
 
@@ -334,9 +342,9 @@ (define_insn "sync_old_add<mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_loop (MIPS_SYNC_OLD_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<d>addu");	
+    return mips_output_sync_loop (MIPS_SYNC_OLD_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -350,7 +358,7 @@ (define_insn "sync_old_sub<mode>"
 	 UNSPEC_SYNC_OLD_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_OLD_OP ("<d>", "<d>subu");	
+  return mips_output_sync_loop (MIPS_SYNC_OLD_OP ("<d>", "<d>subu"));
 }
   [(set_attr "length" "28")])
 
@@ -365,9 +373,9 @@ (define_insn "sync_new_add<mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addiu");	
+    return mips_output_sync_loop (MIPS_SYNC_NEW_OP ("<d>", "<d>addiu"));
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<d>addu");	
+    return mips_output_sync_loop (MIPS_SYNC_NEW_OP ("<d>", "<d>addu"));
 }
   [(set_attr "length" "28")])
 
@@ -381,7 +389,7 @@ (define_insn "sync_new_sub<mode>"
 	 UNSPEC_SYNC_NEW_OP))]
   "GENERATE_LL_SC"
 {
-  return MIPS_SYNC_NEW_OP ("<d>", "<d>subu");	
+  return mips_output_sync_loop (MIPS_SYNC_NEW_OP ("<d>", "<d>subu"));
 }
   [(set_attr "length" "28")])
 
@@ -394,9 +402,9 @@ (define_insn "sync_<optab><mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OP ("<d>", "<immediate_insn>");	
+    return mips_output_sync_loop (MIPS_SYNC_OP ("<d>", "<immediate_insn>"));
   else
-    return MIPS_SYNC_OP ("<d>", "<insn>");	
+    return mips_output_sync_loop (MIPS_SYNC_OP ("<d>", "<insn>"));
 }
   [(set_attr "length" "28")])
 
@@ -411,9 +419,10 @@ (define_insn "sync_old_<optab><mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_OLD_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_OLD_OP ("<d>", "<insn>");	
+    return mips_output_sync_loop (MIPS_SYNC_OLD_OP ("<d>", "<insn>"));
 }
   [(set_attr "length" "28")])
 
@@ -428,9 +437,10 @@ (define_insn "sync_new_<optab><mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>");	
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_NEW_OP ("<d>", "<immediate_insn>")));
   else
-    return MIPS_SYNC_NEW_OP ("<d>", "<insn>");	
+    return mips_output_sync_loop (MIPS_SYNC_NEW_OP ("<d>", "<insn>"));
 }
   [(set_attr "length" "28")])
 
@@ -441,9 +451,9 @@ (define_insn "sync_nand<mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NAND ("<d>", "andi");	
+    return mips_output_sync_loop (MIPS_SYNC_NAND ("<d>", "andi"));
   else
-    return MIPS_SYNC_NAND ("<d>", "and");	
+    return mips_output_sync_loop (MIPS_SYNC_NAND ("<d>", "and"));
 }
   [(set_attr "length" "32")])
 
@@ -456,9 +466,9 @@ (define_insn "sync_old_nand<mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_OLD_NAND ("<d>", "andi");	
+    return mips_output_sync_loop (MIPS_SYNC_OLD_NAND ("<d>", "andi"));
   else
-    return MIPS_SYNC_OLD_NAND ("<d>", "and");	
+    return mips_output_sync_loop (MIPS_SYNC_OLD_NAND ("<d>", "and"));
 }
   [(set_attr "length" "32")])
 
@@ -471,9 +481,9 @@ (define_insn "sync_new_nand<mode>"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_NEW_NAND ("<d>", "andi");	
+    return mips_output_sync_loop (MIPS_SYNC_NEW_NAND ("<d>", "andi"));
   else
-    return MIPS_SYNC_NEW_NAND ("<d>", "and");	
+    return mips_output_sync_loop (MIPS_SYNC_NEW_NAND ("<d>", "and"));
 }
   [(set_attr "length" "32")])
 
@@ -486,9 +496,9 @@ (define_insn "sync_lock_test_and_set<mod
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE ("<d>", "li");
+    return mips_output_sync_loop (MIPS_SYNC_EXCHANGE ("<d>", "li"));
   else
-    return MIPS_SYNC_EXCHANGE ("<d>", "move");
+    return mips_output_sync_loop (MIPS_SYNC_EXCHANGE ("<d>", "move"));
 }
   [(set_attr "length" "24")])
 
@@ -516,8 +526,10 @@ (define_insn "test_and_set_12"
   "GENERATE_LL_SC"
 {
   if (which_alternative == 0)
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP);
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_NONZERO_OP)));
   else
-    return MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP);
+    return (mips_output_sync_loop
+	    (MIPS_SYNC_EXCHANGE_12 (MIPS_SYNC_EXCHANGE_12_ZERO_OP)));
 }
   [(set_attr "length" "28,24")])
Index: gcc/testsuite/gcc.target/mips/fix-r10000-1.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-1.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_fetch_and_add (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_fetch_and_add (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_fetch_and_add (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-2.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-2.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z, int amt)
+{
+  return __sync_fetch_and_sub (z, amt);
+}
+
+NOMIPS16 short
+f2 (short *z, short amt)
+{
+  return __sync_fetch_and_sub (z, amt);
+}
+
+NOMIPS16 char
+f3 (char *z, char amt)
+{
+  return __sync_fetch_and_sub (z, amt);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-3.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-3.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_fetch_and_or (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_fetch_and_or (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_fetch_and_or (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-4.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-4.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_fetch_and_and (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_fetch_and_and (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_fetch_and_and (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-5.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-5.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_fetch_and_xor (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_fetch_and_xor (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_fetch_and_xor (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-6.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-6.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_fetch_and_nand (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_fetch_and_nand (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_fetch_and_nand (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-7.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-7.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_add_and_fetch (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_add_and_fetch (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_add_and_fetch (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-8.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-8.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z, int amt)
+{
+  return __sync_sub_and_fetch (z, amt);
+}
+
+NOMIPS16 short
+f2 (short *z, short amt)
+{
+  return __sync_sub_and_fetch (z, amt);
+}
+
+NOMIPS16 char
+f3 (char *z, char amt)
+{
+  return __sync_sub_and_fetch (z, amt);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-9.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-9.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_or_and_fetch (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_or_and_fetch (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_or_and_fetch (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-10.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-10.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_and_and_fetch (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_and_and_fetch (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_and_and_fetch (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-11.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-11.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_xor_and_fetch (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_xor_and_fetch (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_xor_and_fetch (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-12.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-12.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_nand_and_fetch (z, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_nand_and_fetch (z, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_nand_and_fetch (z, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-13.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-13.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_bool_compare_and_swap (z, 0, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_bool_compare_and_swap (z, 0, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_bool_compare_and_swap (z, 0, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-14.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-14.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  return __sync_val_compare_and_swap (z, 0, 42);
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  return __sync_val_compare_and_swap (z, 0, 42);
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  return __sync_val_compare_and_swap (z, 0, 42);
+}
Index: gcc/testsuite/gcc.target/mips/fix-r10000-15.c
===================================================================
--- /dev/null	2008-11-15 08:55:59.576098500 +0000
+++ gcc/testsuite/gcc.target/mips/fix-r10000-15.c	2008-11-15 14:21:08.000000000 +0000
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-mips-options "-O2 -march=mips4 -mfix-r10000" } */
+/* { dg-final { scan-assembler-times "\tbeql\t" 3 } } */
+
+NOMIPS16 int
+f1 (int *z)
+{
+  int result;
+
+  result = __sync_lock_test_and_set (z, 42);
+  __sync_lock_release (z);
+  return result;
+}
+
+NOMIPS16 short
+f2 (short *z)
+{
+  short result;
+
+  result = __sync_lock_test_and_set (z, 42);
+  __sync_lock_release (z);
+  return result;
+}
+
+NOMIPS16 char
+f3 (char *z)
+{
+  char result;
+
+  result = __sync_lock_test_and_set (z, 42);
+  __sync_lock_release (z);
+  return result;
+}

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Kumba]

Richard Sandiford wrote:
> 
> Yeah, it looks generally good.  I think we've got to the point where
> it's easier for me to make changes directly rather than ask you to
> follow a tortuous list of vaguely-described requests, so:

Tortuous isn't the word I'd use.  More like fun and challenging!  Still a ways 
to go understand things, but it's getting easier to walk around in the mips 
backend of gcc and have an idea of what's going on somewhat.

Thanks for answering all my questions!


>   - I added a missing @gol after "-mfix-r4400".

Yeah, I wasn't sure if I needed that or not.  I was building the info pages and 
comparing the output, and noticed a somewhat logical order where alike arguments 
were grouped onto their own line (i.e., the fix-4000 and fix-4400 were separated 
from the fix-sb1 and fix-vr* args).  Don't know Texi code at all, so I thought 
omitting the @gol would allow the patch to stay in the 80-char limit, but allow 
the info output to place fix-r10000 on the same line as the 4000/4400 args, 
since it appears all three were better related (in terms of implementation) than 
the sb1 and vr* args.


>   - I tweaked the documentation so that it was more consistent with the
>     other -mfix-* options.  Let me know if you spot a problem with the
>     new version, or if you aren't happy with it.

Looks good to me.  I added the bit about MIPS-II stuff and mentioning it won't 
work on MIPS-I, but wasn't sure if that level of detail was necessary.


>   - ...I changed the name of the parameter from TEMPLATE to LOOP to
>     avoid a bootstrap-breaking warning about using a C++ identifier.
>     (Again my fault.  I'd used TEMPLATE when suggesting the function,
>     but it was a completely untested suggestion.)
> 
>   - I added a prototype to mips-protos.h, again to avoid a bootstrap-
>     breaking warning.
> 
>   - I fixed a typo: s/!TARGET_BRANCHLIKEL/!TARGET_BRANCHLIKELY/.
>     GCC wouldn't build without this, so perhaps the posted patch
>     wasn't the final one.

Ah, I probably would have caught these but PR38052 was getting in the way of a 
full build.  Although, building as a cross-compiler might've avoided that bug.


> Applied with those changes.  I've attached the final changelog and
> patch below.  Thanks for the contribution, and for your patience.

Looks good to me.  This'll let me play with the glibc-side of things next.  I 
think I did stumble on one or two omissions from my R10000 scheduling patch I'll 
send over in a separate e-mail (it's a one-line fix I believe).


Thanks again!

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org

"The past tempts us, the present confuses us, the future frightens us.  And our 
lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Sat, 1 Nov 2008, Kumba wrote:

> The branch-likely stuff is only going to work for MIPS-II or higher targets.
> In the odd (but possible) cases where MIPS-I might be used with -mfix-r10000,
> I assume we'll still have to emit 28 nops prior to a beq/beqz instruction.  Is
> this already taken care of someplace?

 MIPS I does not support LL/SC, so that case is not to be concerned.  
These instructions are emulated on such processors and as such can appear 
in assembly snippets, but GCC is expected not to emit them itself (I hope 
it hasn't changed; if it has, then we are in trouble -- I suppose a 
compiler error is justified for such an odd case).

> Hmm, okay.  Might this work to enable -mbranch-likely if -mfix-r10000?  (Kind
> of guessing by looking at other segments of code).
> 
>   if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
>     {
>       if (ISA_HAS_BRANCHLIKELY
>           && (optimize_size
>               || (!(target_flags_explicit & MASK_FIX_R10000) == 0)
>               || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
>         target_flags |= MASK_BRANCHLIKELY;
>       else
>         target_flags &= ~MASK_BRANCHLIKELY;
>     }
> 
> 
> 
> My understanding so far for -mfix-r10000:
> - Gets enabled if -march=r10000 is passed (done)
> - Enable -mbranch-likely if not already enabled on >= MIPS-II (working on)
> - Emits beqzl in the asm templates if enabled and >= MIPS-II (unsure)
> - Emits 28 nops prior to beq/beqz if enabled and == MIPS-I (unsure)
> - Ditto for asm templates (unsure)
> - Documentation (not done)

 I think the best option is to leave -mbranch-likely intact and bail out 
if -mfix-r10000 and -mno-branch-likely are passed at the same time.  
Anything else is likely to cause confusion.  Then -march=r10000 should 
enable both -mfix-r10000 and -mbranch-likely.

 I believe (but have not checked) that all CPUs/ISAs that are within the 
MIPS II - MIPS IV range enable -mbranch-likely by default, so there is no 
problem here unless somebody requests -mno-branch-likely in which case 
they get what they asked for (i.e. a compiler error).  I believe all MIPS 
architecture CPUs/ISAs disable -mbranch-likely by default, but such code 
cannot run on the R10k anyway.

 I think it covers all cases.  Comments?

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Richard Sandiford]

"Maciej W. Rozycki" <macro@linux-mips.org> writes:
> On Sat, 1 Nov 2008, Kumba wrote:
>> The branch-likely stuff is only going to work for MIPS-II or higher targets.
>> In the odd (but possible) cases where MIPS-I might be used with -mfix-r10000,
>> I assume we'll still have to emit 28 nops prior to a beq/beqz instruction.  Is
>> this already taken care of someplace?
>
>  MIPS I does not support LL/SC, so that case is not to be concerned.  
> These instructions are emulated on such processors and as such can appear 
> in assembly snippets, but GCC is expected not to emit them itself (I hope 
> it hasn't changed; if it has, then we are in trouble -- I suppose a 
> compiler error is justified for such an odd case).

Not quite true.  Use of LL and SC is controlled by a separate option,
-mllsc.  As you'd expect, this option is on by default if the architecture
provides LL and SC.  It's therefore on by default for generic MIPS II
and above, and for -march=<cpu> if <cpu> provides the instructions.

However, the default for other architectures depends on the configuration.
If the configured target is known to emulate the LL and SC instructions
(as Linux targets are), the default is -mllsc.  It is -mno-llsc otherwise.

>> Hmm, okay.  Might this work to enable -mbranch-likely if -mfix-r10000?  (Kind
>> of guessing by looking at other segments of code).
>> 
>>   if ((target_flags_explicit & MASK_BRANCHLIKELY) == 0)
>>     {
>>       if (ISA_HAS_BRANCHLIKELY
>>           && (optimize_size
>>               || (!(target_flags_explicit & MASK_FIX_R10000) == 0)
>>               || (mips_tune_info->tune_flags & PTF_AVOID_BRANCHLIKELY) == 0))
>>         target_flags |= MASK_BRANCHLIKELY;
>>       else
>>         target_flags &= ~MASK_BRANCHLIKELY;
>>     }
>> 
>> 
>> 
>> My understanding so far for -mfix-r10000:
>> - Gets enabled if -march=r10000 is passed (done)
>> - Enable -mbranch-likely if not already enabled on >= MIPS-II (working on)
>> - Emits beqzl in the asm templates if enabled and >= MIPS-II (unsure)
>> - Emits 28 nops prior to beq/beqz if enabled and == MIPS-I (unsure)
>> - Ditto for asm templates (unsure)
>> - Documentation (not done)
>
>  I think the best option is to leave -mbranch-likely intact and bail out 
> if -mfix-r10000 and -mno-branch-likely are passed at the same time.  
> Anything else is likely to cause confusion.  Then -march=r10000 should 
> enable both -mfix-r10000 and -mbranch-likely.
>
>  I believe (but have not checked) that all CPUs/ISAs that are within the 
> MIPS II - MIPS IV range enable -mbranch-likely by default, so there is no 
> problem here unless somebody requests -mno-branch-likely in which case 
> they get what they asked for (i.e. a compiler error).  I believe all MIPS 
> architecture CPUs/ISAs disable -mbranch-likely by default, but such code 
> cannot run on the R10k anyway.
>
>  I think it covers all cases.  Comments?

For avoidance of doubt, this is exactly option #1 in my messages.
As you say, -march=r10000 already implies -mbranch-likely unless
explicitly overridden.

Richard

RE: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Paul_Koning]

>From: Maciej W. Rozycki [mailto:macro@linux-mips.org] 
>
> I believe (but have not checked) that all CPUs/ISAs that are within the 
>MIPS II - MIPS IV range enable -mbranch-likely by default, 

Not quite.  sb1 has no-branch-likely.  It actually does implement the instruction but the documentation clearly states that it should be avoided.

	paul

RE: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Paul_Koning]

>From: Maciej W. Rozycki [mailto:macro@linux-mips.org] 
>
> I believe (but have not checked) that all CPUs/ISAs that are within the 
>MIPS II - MIPS IV range enable -mbranch-likely by default, 

Not quite.  sb1 has no-branch-likely.  It actually does implement the instruction but the documentation clearly states that it should be avoided.

	paul

RE: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Mon, 3 Nov 2008, Paul_Koning@Dell.com wrote:

> > I believe (but have not checked) that all CPUs/ISAs that are within the 
> >MIPS II - MIPS IV range enable -mbranch-likely by default, 
> 
> Not quite.  sb1 has no-branch-likely.  It actually does implement the 
> instruction but the documentation clearly states that it should be 
> avoided.

 Well, the SB-1 is a MIPS architecture processor (a MIPS64 one to be 
exact) and as such not within the MIPS II - MIPS IV ISA range and what you 
say by definition stands for any other MIPS architecture implementation 
too.  This is not relevant though as MIPS64 code won't run on a MIPS IV 
processor such as the R10k anyway.

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Ralf Baechle]

On Mon, Nov 03, 2008 at 10:51:49AM -0600, Paul_Koning@Dell.com wrote:

> > I believe (but have not checked) that all CPUs/ISAs that are within the 
> >MIPS II - MIPS IV range enable -mbranch-likely by default, 
> 
> Not quite.  sb1 has no-branch-likely.  It actually does implement the instruction but the documentation clearly states that it should be avoided.

For the usual reasons - the CPU micro architects hate branch likely.  It
means having to cancel an instruction from the pipeline rather late, if
the branch was not taken.  So the deprecation just expresses the desparate
wish of the CPU architects to get rid of the instruction.  In practice
that won't happen any time soon - we even still have useless crap like
signed add and sub instructions in the MIPS ISA.  And if the instructions
actually were removed we still could software emulate them in the kernel.

However unlike the R10000 the SB1 and SB1 implement full blown branch
prediction for branch likely so use it where you can for performance.

  Ralf

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Maciej W. Rozycki]

On Sat, 1 Nov 2008, Richard Sandiford wrote:

> There are two ways we could handle this:
> 
>   - Make -mfix-r10000 require -mbranch-likely.  (It mustn't _imply_
>     -mbranch-likely.  It should simply check that -mbranch-likely is
>     already in effect.)
> 
>   - Make -mfix-r10000 insert nops when -mbranch-likely is not in effect.

 If I recall right, these is something special about the pipeline in this 
context making the branch-likely instructions the only ones that work.  
Which would make the option you proposed first the only viable.  I am not 
absolutely sure and I have no reference handy.  Perhaps Ralf or someone at 
linux-mips will know?

  Maciej

Re: [PATCH]: R10000 Needs LL/SC Workaround in Gcc

[Ralf Baechle]

On Sat, Nov 01, 2008 at 08:33:03PM +0000, Maciej W. Rozycki wrote:

> > There are two ways we could handle this:
> > 
> >   - Make -mfix-r10000 require -mbranch-likely.  (It mustn't _imply_
> >     -mbranch-likely.  It should simply check that -mbranch-likely is
> >     already in effect.)
> > 
> >   - Make -mfix-r10000 insert nops when -mbranch-likely is not in effect.
> 
>  If I recall right, these is something special about the pipeline in this 
> context making the branch-likely instructions the only ones that work.  
> Which would make the option you proposed first the only viable.  I am not 
> absolutely sure and I have no reference handy.  Perhaps Ralf or someone at 
> linux-mips will know?

There are two possible workarounds.  The other which IRIX and the Linux
kernel are using is based on the branch-likely instruction.  The way it
works is that R10000 family processors have a fairly cheesy branch
prediction for branch likely (unlike all MIPS32 and MIPS64 processors I
know of!) which predicts branch likely instructions as always taken.  So
if a SC instruction succeeds the loop closure branch of the usual LL/SC
loop will be miss-predicted and the pipeline restarted.

The alternative is to put enough NOPs (upto 28) after the loop closure
brach to avoid a sequence of 4 problematic instructions being active in
the pipeline at the same time.

SSNOP won't cut it btw.  SSNOP don't have any influence on the predecode
and reordering buffers - even assuming the R10000 actually honors SSNOP.
Implementing the special treatment of SSNOP (which is encoded as
SLL $0, $0, 1) just doesn't make sense for an R10000 calibre processor.

Is gcc capable of guaranteeing a certain minimum number of instructions
between one LL and another LL instruction?  Then this knowledge could be
used to avoid the branch likely or cut down the padding NOPs.

  Ralf