[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