> The atomic-compare-exchange-3.c and atomic-op-3.c tests are failing when
> using the -mfix-r10000 option. This is due to the fact that the delay
> slot of the branch instruction that checks if the atomic operation was
> not successful can be filled with an operation that returns the output
> result. For the branch likely case this operation can not go in the
> delay slot because it wont execute when the atomic operation was
> successful and therefore the wrong result will be returned. This patch
> forces a nop to be placed in the delay slot if a branch likely is used.
> The fix is as simple as possible; it does cause a second nop to be
> introduced
> after the branch instruction in the branch likely case. As this option is
> rarely used, it makes more sense to keep the code readable than trying to
> optimise it.
>
> The atomic tests now pass successfully. The ChangeLog and patch are
> below.
>
> Ok to commit?
I'm OK with just making the fix-r10000 case safe rather than also
complicating the normal code path to remove the normal delay slot NOP in
this special case. I'd like to see what Catherine thinks though. To
remove the second NOP I believe we would have to add a !TARGET_FIX_R10000
to the condition of the normal delay slot NOP. This seems a bit convoluted
when the real reason is because branch likely is in use. To make use of
the mips_branch_likely flag it would have to be set earlier in:
mips_output_sync_loop and also get set in mips_sync_loop_insns.
If Catherine thinks getting rid of the second NOP is important enough then
please base it on mips_branch_likely and fix the callers.
> gcc/
> * config/mips/mips.c (mips_process_sync_loop): Place a nop in the
> delay slot of the branch likely instruction.
>
>
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index 02268f3..6dd3728 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -12997,7 +12997,12 @@ mips_process_sync_loop (rtx_insn *insn, rtx
> *operands)
> This will sometimes be a delayed branch; see the write code below
> for details. */
> mips_multi_add_insn (is_64bit_p ? "scd\t%0,%1" : "sc\t%0,%1", at, mem,
> NULL);
> - mips_multi_add_insn ("beq%?\t%0,%.,1b", at, NULL);
> +
> + /* We can not put the NEWVAL = $TMP3 or CMP = 1 operations in the delay
> slot
> + of the branch if it is a branch likely because they will not execute
> when
> + the atomic operation is successful, so place a NOP in there instead.
> */
> +
I found the comment hard to digest, perhaps:
/* When using branch likely (-mfix-r10000), the delay slot instruction will
be annulled on false. The normal delay slot instructions calculate the
overall result of the atomic operation and must not be annulled.
Unconditionally use a NOP instead for the branch likely case. */
> + mips_multi_add_insn ("beq%?\t%0,%.,1b%~", at, NULL);
>
> /* if (INSN1 != MOVE && INSN1 != LI) NEWVAL = $TMP3 [delay slot]. */
> if (insn1 != SYNC_INSN1_MOVE && insn1 != SYNC_INSN1_LI && tmp3 !=
> newval)
Matthew
