[cfe-commits] [PATCH] atomic operation builtins, part 1

[Eli Friedman]

On Tue, Oct 11, 2011 at 6:15 PM, Andrew MacLeod <amacleod at redhat.com> wrote:
> On 10/07/2011 08:03 PM, Eli Friedman wrote:
>>>
>>> I believe gcc is using __sync_mem_* instead of __atomic_* for their
>>> builtins, although they're using __atomic_* for the library that
>>> handles maybe-not-lock-free calls:
>>> http://gcc.gnu.org/wiki/Atomic/GCCMM/LIbrary.  I've cc'ed Andrew
>>> MacLeod who's actually deciding this, in the hope that both compilers
>>> can use the same intrinsics.
>>
>> Okay, I can wait to hear back.
>
> Actually, Im renaming the __sync_mem routines to be __atomic to match the
> external routines.  They were originally __sync_mem because they built on
> the original __sync routines, but I think we're better off long term with
> the __atomic prefix to match everything up.
>
> I've also decided after thinking about it during the weekend (it was a long
> weekend here in canada... sorry for the delays getting back to you guys), to
> tweak and simplify the GCC intrinsic interface slightly.   (well, thats my
> plan right now anyway, this week will tell the tale :-)
>
> The intrinsics and the external library don't match up exactly, I don't know
> if thats a problem or not.

That seems like the right approach.

> Is your intent/hope/desire to have GCC and
> llvm have the exact same intrinsics as well as call the same library
> routines?

Making the library routines match is necessary for ABI compatibility.
Making the intrinsics the same isn't a hard requirement, but making
them different seems like it would just generate extra work for
everyone.

On another random ABI note, do you plan on supporting lock-free
operations on types whose ABI alignment is less than their size?  I'm
guessing you're planning on requiring alignment equal to the size for
atomic operations on types with a small power-of-two size regardless
of the ABI alignment; otherwise, the proposed signature of
__atomic_is_lock_free won't work.  What about for types whose size is
small and not a power of two?

> I've updated the last section of that wiki page to explicitly lay out what
> I'm thinking:
>
> http://gcc.gnu.org/wiki/Atomic/GCCMM/LIbrary#GCC_intrinsics

The one substantial difference here between your proposal and what I
am currently implementing is the type of the pointer argument.  In my
current implementation, it has type _Atomic(T)* (where _Atomic comes
from the C1x specification). This encodes in the type system the
requirement that the argument must be an atomic variable
(appropriately aligned, etc.).  Sort of related to the ABI questions
above.

The other differences are minor: my current implementation has
signatures like "T __atomic_load(_Atomic(T)*, int)" for atomic load,
etc. and there are separate __atomic_compare_exchange_weak and
__atomic_compare_exchange_strong.  Easy to change either way.

>>> This should probably have triggered an error earlier in the frontend.
>>> If that's impossible, comment why?
>>
>> We don't actually check the value of the order argument in Sema
>> because it can be a variable... so an assert here would be fragile.  I
>> can add a comment.
>
> Which I find incredibly annoying.  GCC has currently taken the approach that
>  a) if the memory_order parameter turns out not to be a compile time
> constant, and
>  b) it *is* always lock free (meaning it can generate instructions rather
> than calls)
> we simply inline the operation using memory_order_seq_cst rather than having
> to go through a switch to figure out which one.
>
> Maybe that will be reverted down the road, but my current take is that its
> not something that is going to happen very often, and when it does, it's
> probably not very critical.  Its easy to revert later if it can be
> demonstrated that its worthwhile.

Okay, good to know.  At the moment, my implementation doesn't do that
simply because it inlines the operation into LLVM IR before function
inlining.

-Eli