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

David Daney ddaney@avtrex.com
Thu Sep 13 01:15:00 GMT 2007


Hans-Peter Nilsson wrote:
> 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

My understanding is that (set (mem:BLK (scratch)) ...) has the same 
effect as unspec_volatile, making the volatile part redundant.  I'm sure 
someone will correct me if I am wrong.

> (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.  :)

The problem is that there is very little code that uses the MIPS 
memory_barrier (libgcj mostly).

David Daney



More information about the Gcc-patches mailing list