Re: __sync_swap* with acq/rel/full memory barrier semantics

[Andrew MacLeod <amacleod@redhat.com>]

> On 06/17/2011 02:12 PM, Andrew MacLeod wrote:
>> --- machmode.h	(working copy)
>> *************** extern enum machine_mode ptr_mode;
>> *** 275,278 ****
>> --- 275,291 ----
>>    /* Target-dependent machine mode initialization - in insn-modes.c.  */
>>    extern void init_adjust_machine_modes (void);
>>
>> + /* Memory model types for the __sync_mem* builtins.
>> +    This must match the order in libstdc++-v3/include/bits/atomic_base.h.  */
>> + enum memmodel
>> + {
>> +   MEMMODEL_RELAXED = 0,
>> +   MEMMODEL_CONSUME = 1,
>> +   MEMMODEL_ACQUIRE = 2,
>> +   MEMMODEL_RELEASE = 3,
>> +   MEMMODEL_ACQ_REL = 4,
>> +   MEMMODEL_SEQ_CST = 5,
>> +   MEMMODEL_LAST = 6
>> + };
> This isn't a very machine mode sort of define.
> I think coretypes.h is a better choice.

cool that seems to work fine.  As long as its somewhere common.

>> + static rtx
>> + expand_builtin_mem_exchange (enum machine_mode mode, tree exp, rtx target)
> Some names include "sync" and some don't?

Well, I was going to blame Aldy :-)  but then I went to look at this, 
and thats the same way *all* the other __sync instructions seem to be.

ie:

builtins.c:expand_builtin_lock_test_and_set (enum machine_mode mode, 
tree exp,
builtins.c:    case BUILT_IN_LOCK_TEST_AND_SET_1:
builtins.c:    case BUILT_IN_LOCK_TEST_AND_SET_2:
builtins.c:    case BUILT_IN_LOCK_TEST_AND_SET_4:

whereas everything else is 'sync_lock_test_and_set'..

So i guess it falls to prior art... I assume Aldy just cut-and-pasted 
for his new routine and just changed the names in the same format.


>> + 
> The xchg instruction is a full barrier; no need for anything extra here.
> Indeed, you needn't define UNSPECV_MEM_XCHG either.  This could be as
> simple as
>
>
Ah, even better.  For some reason I thought I saw somewhere that it 
wasn't a full barrier.  Might have just been the documentation for 
lock_test_and_set.

Andrew