[PATCH] D123642: [clang codegen] Assume arguments of __atomic_* are aligned.

James Y Knight via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 12 20:11:09 PDT 2022


jyknight added a comment.

I disagree with this on principle -- IMO, it is basically a historical bug in GCC that it ignores the type alignment, and we should NOT try to match that -- it's dangerous.

We ought to resolve the bug via other fixes:

- As a workaround: add alignas(uint64_t) to the affected struct in lld. (is GHashCell the only one?)
- Fix `__builtin_addressof` in Clang to propagate alignment in the same way `&` does (they ought to act equivalently; I think a new stanza in CodeGenFunction::EmitPointerWithAlignment will fix that).
- Ask GCC to modify libstdc++ so that `__builtin_addressof` is called directly, instead of going through `std::__addressof`.

In addition to not liking the intended change, I think this implementation is wrong -- and will be treacherous to attempt to fix -- because I'm pretty sure GCC's implementation doesn't assume particular alignments in general; if it decides it _cannot_ emit inline atomics (based on the size and target's configuration), then no extra alignment is assumed. (And note that Clang's idea of MaxPromoteWidth is unrelated to which sizes of atomics GCC inlines.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123642



More information about the cfe-commits mailing list