jyknight requested changes to this revision.
jyknight added a comment.
This revision now requires changes to proceed.

This isn't correct.

The atomic interface is designed to be forward-compatible with new CPUs that 
have wider atomic instructions. That clang has a MaxAtomicPromoteWidth value 
_at all_ is unfortunate, because it breaks this ability. We've locked ourselves 
into an unfortunate ABI here -- and what's worse, it's incompatible with GCC's 
ABI.

But, the MaxAtomicPromoteWidth value impactsaffects C11 _Atomic (and therefore 
libc++ std::atomic) variables' default alignment. But, the user can in any case 
specify additional alignment. For the __atomic_is_lock_free function, 
MaxAtomicPromoteWidth is irrelevant -- it can be used on variables of any size 
or alignment and needs to return correct answers, even on future CPUs.

It's not good that clang doesn't provide the ability to create a working 
libatomic, but the right answer is to fix that, not add hacks like this. I've 
started writing up a proposal on what we need to do there, but need to finish 
that up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72579/new/

https://reviews.llvm.org/D72579



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D72579: Evaluate __... Fangrui Song via Phabricator via cfe-commits
    • [PATCH] D72579: Evalua... James Y Knight via Phabricator via cfe-commits

Reply via email to