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
16 matches
Mail list logo
