> From: Maciej W. Rozycki [mailto:ma...@codesourcery.com] > > 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