[Patch] MIPS: Add missing memory clobbers to atomic memory insns.

[Richard Sandiford]

Hans-Peter Nilsson <hp@bitrange.com> writes:
> On Sun, 9 Sep 2007, Richard Sandiford wrote:
>> David Daney <ddaney@avtrex.com> writes:
>> The idea looks sound, but...
>>
>> > -(define_expand "memory_barrier"
>> > -  [(unspec_volatile [(const_int 0)] UNSPEC_SYNC)]
>> > +(define_insn "memory_barrier"
>> > +  [(set (mem:BLK (scratch))
>> > +        (unspec_volatile:BLK [(const_int 0)] UNSPEC_MEMORY_BARRIER))]
>> >    "ISA_HAS_SYNC"
>> > -  "")
>> > +  "sync")
>>
>> ...the insn is doing nothing other than acting as a memory barrier,
>> so I don't think it needs to be unspec_volatile after this change.
>> A plain unspec should do.  gcc should still add the required dependencies
>> for other memory operations, but the sync really does have no dependencies
>> on preceding arithmetic ops.
>
> My experience with unspec_volatile involved adding one as a
> stack-access blocker for the stack-pointer adjustment in the
> function epilogue for CRIS, see "cris_frame_deallocated_barrier".
> I had to make it unspec_volatile, as unspec wouldn't stop gcc
> from moving stack-accessing insns e.g. into the delay-slot of
> the return insn (IIRC).  Maybe that doesn't happen here because
> of the (mem:BLK (scratch)) destination (I used (mem:BLK (reg:SI
> CRIS_SP_REGNUM))), but if you're "gcc should" not "gcc will",
> let me suggest keeping an eye on this and perhaps checking the
> corresponding differences on a large amount of code (sorry, not
> volunteering.  :)

Sorry, I was being overly cautious saying "gcc should".  "gcc should
in principle" if you like; in other words, if gcc doesn't treat
(mem:BLK (scratch)) as aliasing everything, that's a bug, but I can't
guarantee that no such bug exists.  This is the construct that we use
to implement asm memory clobbers, so it must be made to work.

See files like alias.c and dse.c for why (mem:BLK (reg:SI CRIS_CP_REGNUM))
is different from (mem:BLK (scratch)).

Richard