[cxx-mem-model] __sync_mem builtin support patch 2/3 - code

Richard Henderson rth@redhat.com
Wed Jul 27 21:40:00 GMT 2011


On 07/27/2011 11:17 AM, Andrew MacLeod wrote:
> On 07/27/2011 12:03 PM, Richard Henderson wrote:
>> Please disable the relevant tests too.
> sure.
> 
>>>      if ((icode != CODE_FOR_nothing)&&  (model == MEMMODEL_SEQ_CST ||
>>>                         model == MEMMODEL_ACQ_REL))
>>> + #ifdef HAVE_sync_mem_thread_fence
>>> +     emit_mem_thread_fence (model);
>>> + #else
>>>        expand_builtin_sync_synchronize ();
>>> + #endif
>> Coding style requires braces here.  Yes, only one of the two
>> functions are called, but that's not immediately obvious to
>> the eye.
>>
>> Lots of other instances in your new code.
>>
>> That said, why wouldn't emit_mem_thread_fence always exist
>> and generate the expand_builtin_sync_synchronize as needed?
> 
> Done after a chat with you sorting it out.. I added an expand_builtin_mem_thread_fence() routine which does just this, much cleaner :-)
> 
> I also noticed that all the expand_builtin_sync_mem_ flag and fence routines (4 in total) were all not quite correct. none of them would actually use a pattern if it was defined, so I changed them a bit to do that.  They were basically the set which did not have a TYPE modifier, so didnt have entries in the direct_optab table, so were missing out.
> 
> Rest is done.  Patch is attached in case you want to look at the changes.

Looks ok.


r~



More information about the Gcc-patches mailing list