[PATCH] X86: rework expansion of atomic instructions

[Chandler Carruth]

LGTM.

On Tue, Jul 1, 2014 at 2:37 AM, Tim Northover <t.p.northover at gmail.com>
wrote:

> Hi Chandler,
>
> Thanks for looking at the patch!
>
> > This has the potential to be an ABI-breaking change. Does your new pass
> fall
> > back to libcalls in exactly the same places that the old code did?
> Otherwise,
> > depending on how the libctalls are implemented, this could cause a break
> with
> > code compiled previously.
>
> That's a difficult one. It *does* change when we fall back, but
> because of how it happens I think it's probably OK for a variety of
> reasons:
>
> 1. The helpers aren't actually implemented on either Linux or OS X.
> GCC already inlines __int128 atomic ops so it's arguably *fixing* an
> ABI bug.
>

I think this is actually the critical thing that makes this a strict
improvement.


>
> 2. We already inline atomic_compare_exchange (on 128-bit types) to
> cmpxchg16b when available; this expands the inlining to other
> atomicrmw operations. Since these can be mixed, I think any ABI
> implementation we're compatible with already has to implement these
> calls in terms of cmpxchg16b rather than any global lock. So it
> probably doesn't break currently unbroken ABIs.
>
> 3. The change is in the backend, and Clang has its own
> MaxAtomicInlineWidth override. It seems a reasonable position that if
> you're using LLVM's atomic instructions, you're requesting inlining if
> possible.
>

Clang and C++'s ABI concerns are totally separate, I agree. I'm not worried
on that front.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140701/5f1b7bee/attachment.html>