diff mbox

If using branch likelies in MIPS sync code fill the delay slot with a nop

Message ID 0DA23CC379F5F945ACB41CF394B9827720F3431A@LEMAIL01.le.imgtec.org
State New
Headers show

Commit Message

Andrew Bennett Nov. 19, 2014, 3:05 p.m. UTC 
> Please rephrase the comment along the lines of my previous suggestion.
> This wording is too complex IMO.

The patch containing the updated comment (which also keeps within 72 columns)
is below.

Ok to commit?

Regards,


Andrew

Comments

Matthew Fortune Nov. 20, 2014, 11:27 a.m. UTC | #1 
> Ok to commit?

> gcc/
> 	* config/mips/mips.c (mips_process_sync_loop): Place a nop in the 
> 	delay slot of the branch likely instruction.

With an updated ChangeLog to account for the changes in the callers, OK.

Matthew
Richard Sandiford Nov. 26, 2014, 4:03 p.m. UTC | #2 
FWIW, I agree this is the right fix, but:

Andrew Bennett <Andrew.Bennett@imgtec.com> writes:
> +  /* 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.  To ensure this behaviour unconditionally use a NOP
> +     in the delay slot for the branch likely case.  */

remember to use US spelling: "behavior" rather than "behaviour".

Thanks,
Richard
diff mbox

Patch

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 02268f3..bf5682c 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -12997,7 +12997,14 @@  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);
+
+  /* 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.  To ensure this behaviour unconditionally use a NOP
+     in the delay slot 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)
@@ -13005,7 +13012,7 @@  mips_process_sync_loop (rtx_insn *insn, rtx *operands)
       mips_multi_copy_insn (tmp3_insn);
       mips_multi_set_operand (mips_multi_last_index (), 0, newval);
     }
-  else if (!(required_oldval && cmp))
+  else if (!(required_oldval && cmp) && !mips_branch_likely)
     mips_multi_add_insn ("nop", NULL);
 
   /* CMP = 1 -- either standalone or in a delay slot.  */
@@ -13029,12 +13036,12 @@  mips_process_sync_loop (rtx_insn *insn, rtx *operands)
 const char *
 mips_output_sync_loop (rtx_insn *insn, rtx *operands)
 {
-  mips_process_sync_loop (insn, operands);
-
   /* Use branch-likely instructions to work around the LL/SC R10000
      errata.  */
   mips_branch_likely = TARGET_FIX_R10000;
 
+  mips_process_sync_loop (insn, operands);
+
   mips_push_asm_switch (&mips_noreorder);
   mips_push_asm_switch (&mips_nomacro);
   mips_push_asm_switch (&mips_noat);
@@ -13056,6 +13063,9 @@  mips_output_sync_loop (rtx_insn *insn, rtx *operands)
 unsigned int
 mips_sync_loop_insns (rtx_insn *insn, rtx *operands)
 {
+  /* Use branch-likely instructions to work around the LL/SC R10000
+     errata.  */
+  mips_branch_likely = TARGET_FIX_R10000;
   mips_process_sync_loop (insn, operands);
   return mips_multi_num_insns;
 }