[PATCH] X86: rework expansion of atomic instructions

Tim Northover t.p.northover at gmail.com
Tue Jul 1 02:37:42 PDT 2014


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.

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.

So I think it's a positive change regardless. And if we do want to go
back to helpers, we should probably do it in Clang (I'd be happy to
get the patch together to do that if I've not managed to convince
you).

Meanwhile, I'll update the patch for your other comments.

Cheers.

Tim.

http://reviews.llvm.org/D4160






More information about the llvm-commits mailing list