Looks good, thanks. Just a couple of silly comments... Maxim Kuvyrkov <ma...@codesourcery.com> writes: > +/* Subroutines of the mips_process_sync_loop. > + Emit barriers as needed for the memory MODEL. */ > + > +static bool > +mips_emit_pre_atomic_barrier_p (enum memmodel model) > +{ > + switch (model) > + { > + case MEMMODEL_RELAXED: > + case MEMMODEL_CONSUME: > + case MEMMODEL_ACQUIRE: > + return false; > + case MEMMODEL_RELEASE: > + case MEMMODEL_ACQ_REL: > + case MEMMODEL_SEQ_CST: > + return true; > + default: > + gcc_unreachable (); > + } > +}
Comment is a bit misleading because we don't emit anything here. How about: /* Subroutine of mips_process_sync_loop. Return true if memory model MODEL requires a pre-loop (release-style) barrier. */ > +static bool > +mips_emit_post_atomic_barrier_p (enum memmodel model) > +{ > + switch (model) > + { > + case MEMMODEL_RELAXED: > + case MEMMODEL_CONSUME: > + case MEMMODEL_RELEASE: > + return false; > + case MEMMODEL_ACQUIRE: > + case MEMMODEL_ACQ_REL: > + case MEMMODEL_SEQ_CST: > + return true; > + default: > + gcc_unreachable (); > + } > +} /* Subroutine of mips_process_sync_loop. Return true if memory model MODEL requires a post-SC (acquire-style) barrier. */ > + /* CMP = 0 [delay slot]. */ > + if (cmp) > + mips_multi_add_insn ("li\t%0,0", cmp, NULL); Nitlet, but only one space after "CMP" (here and elsewhere). > +(define_expand "atomic_exchange<mode>" > + [(match_operand:GPR 0 "register_operand") > + (match_operand:GPR 1 "memory_operand") > + (match_operand:GPR 2 "arith_operand") > + (match_operand:SI 3 "const_int_operand")] > + "GENERATE_LL_SC" > +{ > + emit_insn (gen_atomic_exchange<mode>_llsc (operands[0], operands[1], > + operands[2], operands[3])); > + DONE; > +}) Excess indentation on "emit_insn" call. Same for atomic_fetch_add<mode>. Richard