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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
