[PATCH] D78810: [InstCombine] Check max alignment before adding attr on aligned_alloc

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula added a comment.

In D78810#2001925 <https://reviews.llvm.org/D78810#2001925>, @lebedev.ri wrote:

> Thank you for looking into this.
>  This should instead clamp to the maximal alignment. (the 'is power of 2' check should remain)


Clamping would be incorrect here since the allocation risks not providing the desired alignment if it gets promoted to alloca. aligned_alloc doesn't appear to have an upper bound on ailgnment (any power of 2 size_t value is fine I think), while the 1 << 29 limit is for LLVM's alloca among others. I think the right/better fix for this could be to just add the attribute to aligned_alloc as long as it's a power of two, but not promote to alloca when it's larger than 1 << 29.  But then I'm not sure if that would cause other undesired behavior in LLVM with an alignment attribute more than 1 << 29. If it did, then not adding the attribute at all for larger than MaximumAlignment appears to be the safe/right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78810