[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