[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

> For C++, I confess I have some problems interpreting this sentence:
>
> (https://eel.is/c++draft/cpp.predefined)
>
>> `__STDCPP_­DEFAULT_­NEW_­ALIGNMENT__`:  An integer literal of type std::size_t whose value is the alignment guaranteed by a call to operator new(std::size_t) or operator new[](std::size_t). [ Note: Larger alignments will be passed to operator new(std::size_t, std::align_val_t), etc. (8.3.4). — end note ]
>
> It seems to suggest that a large alignment may be assumed, but practically, I know at least mimalloc uses 8-byte alignment for 64-bit Darwin, not 16-byte alignment.

See https://eel.is/c++draft/basic.stc.dynamic.allocation#3 (this text was moved around a bit since C++17, but it hasn't changed meaning). `operator new(std::size_t)` requires that "the storage is aligned for any object that does not have new-extended alignment and is of the requested size." ()That is, if you have a size 8 allocation, it is not required to be 16-byte aligned.

> [...] This needs to be very careful since `__STDCPP_­DEFAULT_­NEW_­ALIGNMENT__` doesn't seem to match the practice.

I don't think that's true? I believe that macro is generally being set / handled correctly. Reintroducing an optimization like this with an additional check that the allocation size is large enough should be valid everywhere.

> mimalloc uses 8-byte alignment for 64-bit Darwin, not 16-byte alignment.

It uses 8-byte alignment for 8-byte allocations, you mean. It uses 16-byte alignment for 16-byte (or larger) allocations (doing otherwise would be a clear bug, given that max_align_t is 16).

In D118804#3291060 <https://reviews.llvm.org/D118804#3291060>, @xbolva00 wrote:

> Or can we bailout just for gnu? and preserve this for darwin?

This isn't a GNU-only issue, so I don't think that makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118804