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

["Moore, Catherine" <Catherine_Moore at mentor dot com>]

> -----Original Message-----
> From: Matthew Fortune [mailto:Matthew.Fortune@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
> slot with a nop
> 
> > 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.
> 
  Yes, removing the second NOP is worth the additional effort.