[PATCH] D18201: Switch over targets to use AtomicExpandPass, and clean up target atomics code.

[Tim Northover via llvm-commits]

Hi James,

On 16 March 2016 at 18:17, James Y Knight <jyknight at google.com> wrote:
> So the DAG expansions would only come into play if you somehow had a platform which claims (via setMaxAtomicSizeSupported) backend support for that size, but doesn't even have an atomic load or store instruction capable of that size. I know of no platform where that'd be possibly useful. Certainly none that Darwin supports.

I believe x86 falls into this category for 128-bit types. 8.1.1 of
Volume 3[1] gives some guarantees for normal operations up to 64-bits,
but says nothing about 128.

To get 128-bit guarantees you seem to need a LOCK prefix, which can't
be legally applied to a MOV (but can to cmpxchg16b and xchg). Even the
cmpxchg16b documentation says that "The processor never produces a
locked read without also producing a locked write" which, while not
directly applicable to MOV, is suggestive.

Finally, this is how GCC's libatomic actually implements these
operations, except less buggily (it seems Clang doesn't actually take
account of that cmpxchg16b note, so atomic loads are probably broken
now).

The broken state of Clang might be a get-out, but not trivially:

  * We can't expand to __atomic_load_16 because we can't rely on the
correct .dylib being present for a long time.
  * We can't expand to __sync_load because it doesn't exist.

So the alternatives seem to be a new __sync_load & __sync_store (when
everything else uses cmpxchg16b) or to keep (and fix[2]) the existing
expansions. I think I'd prefer the latter.

Cheers.

Tim.

[1] https://www-ssl.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf
[2] I'm happy to do that after you're done; please don't think I'm
trying to foist fixing more of (probably) my bugs on you.