Kumba <kumba@xxxxxxxxxx> 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
