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

[Tim Northover via llvm-commits]

t.p.northover added a comment.

Ugh, bloody Phabricator! This is what I meant to say:

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.


http://reviews.llvm.org/D18201