[PATCH 1/3] Add atomic_compare_and_swap, atomic_exchange and atomic_fetch_add patterns.
Richard Sandiford
rdsandiford@googlemail.com
Wed Jun 13 18:50:00 GMT 2012
Looks good, thanks. Just a couple of silly comments...
Maxim Kuvyrkov <maxim@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
More information about the Gcc-patches
mailing list