> From: Maciej W. Rozycki [mailto:[email protected]]
>
> On Tue, 18 Nov 2014, Andrew Bennett wrote:
>
> > Produces (for the atomic operation):
> >
> > .set noat
> > 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 $1,$0,1b
> > nop
> > nop
> > sync
> > 2:
> > .set at
>
> 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:
Correct.
>
> else if (!(required_oldval && cmp) && !TARGET_FIX_R10000)
> mips_multi_add_insn ("nop", NULL);
>
> instead? It appears plain and simple, and if you're concerned about it
> being unobvious to the casual reader, then add a one-line comment or
> suchlike. It's not like TARGET_FIX_R10000 is going to imply anything
> else about branches in the future (and r6 code won't run on an R10k
> anyway).
True. Actually this is what my original patch had in it, but Matthew and I
decided to leave it out for the initial submission. I am happy to add
it back in, and provide a comment to explain what is happening.
Catherine are you happy with this?
Regards,
Andrew
