On 14/06/2012, at 6:33 AM, Richard Sandiford wrote:
> Looks good, thanks. Just a couple of silly comments...
>
> Maxim Kuvyrkov <[email protected]> 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.
Yeap, forgot to update the comment after stealing the code from Alpha's port.
> 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).
OK.
>
>> +(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>.
Not if you consider it together with the second patch ;-).
--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics
