jyknight added a comment. > The idea here is not to "ignore type alignment". `EmitPointerWithAlignment` > will sometimes return an alignment for a pointer that's less than the > alignment of the pointee type, e.g. because you're taking the address of a > packed struct field. The critical question is whether the atomic builtins > ought to make an effort to honor that reduced alignment, even if it leads to > terrible code, or if we should treat the use of the builtin as a user promise > that the pointer is actually more aligned than you might think from the > information statically available.
Agreed -- that is the question. In general, I'd like to default to basing decisions only on the statically-known alignments, because I think that'll typically be the best choice for users. Where there's something like a packed struct, it's likely that the values will end up under-aligned in fact, not just in the compiler's understanding. > For example, the MSDN documentation for `InterlockedIncrement` says that it > requires 32-bit alignment [...]. I would say that all of the Interlocked > APIs ought to be read as guaranteeing the natural, full-width alignment for > their operation. I had missed that it was documented in some of the other functions (beyond InterlockedCompareExchange128). I'll change the remainder of the _Interlocked APIs to assume at least natural alignment. > 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97224/new/ https://reviews.llvm.org/D97224 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
