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

Yuanfang Chen via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 2 16:30:13 PST 2022


ychen added a comment.

In D118804#3292263 <https://reviews.llvm.org/D118804#3292263>, @jyknight wrote:

> In D118804#3292185 <https://reviews.llvm.org/D118804#3292185>, @ychen wrote:
>
>> I don't see why the patch is wrong... It uses the target/platform-specific `NewAlign`. If the platform allows customized memory allocation that assumes weak alignment, it should set the `NewAlign` accordingly, no?
>
> NewAlign is set to the largest object alignment for which the compiler can call `::operator new(size_t)`, instead of calling `::operator new(size_t, align_t)`, so, no, you definitely wouldn't want to change that value.
>
> That is: it is correctly set to 16 if `::operator new(16)`` will return 16-byte-aligned memory, even if `::operator new(8)` will return an 8-byte-aligned memory. That's because 8-byte-aligned memory is "suitably-aligned" for any possible object that can fit in 8 bytes.

If I understand it correctly, this is assuming a malloc that could return a different alignment depending on the allocation size. Some other malloc always returns the same alignment (which the reverted patch has assumed). Yeah, then the reverted patch needs to do it in a platform-specific way (default opt-out?).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118804



More information about the cfe-commits mailing list