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

[Richard Sandiford <rdsandiford@googlemail.com>]

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