FWIW, I agree this is the right fix, but:
Andrew Bennett 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 and must not
> + be
> 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
> 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
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
> 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 w
> 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 +12997,1
> -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
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 j
> > 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
> 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
> >
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)
>
> 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
> d
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
> 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
> res
> -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 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 wi
Hi,
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
17 matches
Mail list logo
