[PATCH] D97224: Use Address for CGBuilder's CreateAtomicRMW and CreateAtomicCmpXchg.
James Y Knight via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 12 09:30:56 PST 2021
jyknight added a comment.
In D97224#2621328 <https://reviews.llvm.org/D97224#2621328>, @rjmccall wrote:
> In D97224#2604537 <https://reviews.llvm.org/D97224#2604537>, @jyknight wrote:
>
>>> I'm less certain about what to do with the `__atomic_*` builtins
>>
>> The `__atomic` builtins have already been supporting under-aligned pointers all along -- and that behavior is unchanged by this patch.
>
> How so? Clang hasn't been passing down an alignment, which means that it's been building `atomicrmw` instructions with the natural alignment for the IR type, which means we've been assuming that all pointers have a least that alignment. The LLVM documentation even says that `atomicrmw` doesn't allow under-alignment.
We construct a libcall to `__atomic_*` routines in the frontend upon seeing an underaligned argument, instead of letting the backend handle it -- there's a bunch of code at https://github.com/llvm/llvm-project/blob/bc4a5bdce4af82a5522869d8a154e9e15cf809df/clang/lib/CodeGen/CGAtomic.cpp#L790 to handle that. I'd like to rip most of that out in the future, and just let the backend handle it in more cases.
E.g.
typedef int __attribute__((aligned(1))) unaligned_int;
bool cmpxchg_u(unaligned_int *x) {
int expected = 2;
return __atomic_compare_exchange_n(x, &expected, 2, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
}
generates a libcall to `__atomic_compare_exchange_4` (in IR, generated in the Clang code), instead of creating a cmpxchg IR instruction, due to the under-alignment. (Although, sigh, I've only just noticed we actually have a problem here -- the `__atomic_*_SIZE` libcalls are supposed to require an aligned argument -- so we should be using `__atomic_compare_exchange` (without size suffix) instead. Gah.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97224/new/
https://reviews.llvm.org/D97224
More information about the cfe-commits
mailing list