Re: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-26 Thread Richard Sandiford
FWIW, I agree this is the right fix, but: Andrew Bennett andrew.benn...@imgtec.com writes: + /* When using branch likely (-mfix-r1), the delay slot instruction + will be annulled on false. The normal delay slot instructions + calculate the overall result of the atomic operation

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-20 Thread Matthew Fortune
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

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-19 Thread Moore, Catherine
-Original Message- From: Matthew Fortune [mailto:matthew.fort...@imgtec.com] Sent: Tuesday, November 18, 2014 9:42 AM To: Andrew Bennett; gcc-patches@gcc.gnu.org Cc: Moore, Catherine; Rozycki, Maciej Subject: RE: [PATCH] If using branch likelies in MIPS sync code fill the delay

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-19 Thread Andrew Bennett
Yes, removing the second NOP is worth the additional effort. The updated patch is below. Ok to commit? Regards, Andrew diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 02268f3..368c6f0 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -12997,7

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-19 Thread Matthew Fortune
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 02268f3..368c6f0 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

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-19 Thread Maciej W. Rozycki
On Wed, 19 Nov 2014, Matthew Fortune wrote: @@ -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,

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-19 Thread Andrew Bennett
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 diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index

Re: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Maciej W. Rozycki
On Tue, 18 Nov 2014, Andrew Bennett wrote: The atomic-compare-exchange-3.c and atomic-op-3.c tests are failing when using the -mfix-r1 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

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Andrew Bennett
-Original Message- From: Maciej W. Rozycki [mailto:ma...@codesourcery.com] Sent: 18 November 2014 13:48 To: Andrew Bennett Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop On Tue, 18 Nov 2014, Andrew

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Matthew Fortune
The atomic-compare-exchange-3.c and atomic-op-3.c tests are failing when using the -mfix-r1 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.

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Maciej W. Rozycki
On Tue, 18 Nov 2014, Andrew Bennett wrote: My fix places a nop in the delay slot of the branch likely instruction by using the %~ output operation. This then causes the sync code for the previous example to be correct: .setnoat sync # 15 sync_new_addsi/2

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Andrew Bennett
OK, this does look to me like the correct way to address the issue, but where is the second NOP that you previously mentioned? I fail to see it here and this code can't be made any better, there isn't anything you could possibly schedule into the delay slot as there is nothing else to do in

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Maciej W. Rozycki
On Tue, 18 Nov 2014, Andrew Bennett wrote: Produces (for the atomic operation): .setnoat sync 1: ll $3,0($5) and $1,$3,$4 bne $1,$7,2f and $1,$3,$6 or $1,$1,$8 sc $1,0($5) beql

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Andrew Bennett
From: Maciej W. Rozycki [mailto:ma...@codesourcery.com] On Tue, 18 Nov 2014, Andrew Bennett wrote: Produces (for the atomic operation): .setnoat sync 1: ll $3,0($5) and $1,$3,$4 bne $1,$7,2f and $1,$3,$6

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Matthew Fortune
From: Maciej W. Rozycki [mailto:ma...@codesourcery.com] On Tue, 18 Nov 2014, Andrew Bennett wrote: Produces (for the atomic operation): .setnoat sync 1: ll $3,0($5) and $1,$3,$4 bne $1,$7,2f and

RE: [PATCH] If using branch likelies in MIPS sync code fill the delay slot with a nop

2014-11-18 Thread Maciej W. Rozycki
On Tue, 18 Nov 2014, Matthew Fortune wrote: With a quick look at `mips_process_sync_loop' it looks to me the other NOP is produced from here: else if (!(required_oldval cmp)) mips_multi_add_insn (nop, NULL); -- correct? If so, then can't you just make it: