[PATCH] D115888: [Attributor][Fix] Add default alignment to HeapToStack

Thu Dec 16 13:40:47 PST 2021

jhuber6 added a comment.

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

> In D115888#3198322 <https://reviews.llvm.org/D115888#3198322>, @jhuber6 wrote:
>
>> In D115888#3198178 <https://reviews.llvm.org/D115888#3198178>, @lebedev.ri wrote:
>>
>>> Then i guess you need to basically introduce an interface to do what https://en.cppreference.com/w/cpp/types/max_align_t does, but based on a datalayout.
>>
>> Seems reasonable. I know we can query this information from clang, e.g. https://clang.llvm.org/doxygen/classclang_1_1TargetInfo.html#a01403a5106161d4d3cd0c50c43150f89, but I don't think there is an existing string in the data layout to encode this. Will I be adding a new format this? That would be a reasonably large change so I just want to make sure I'm on the right page.
>
> Err, no. I'm simply thinking that datalayout already specifies the primitive [scalar] types, so you should just need to go through them and pick the one with maximal alignment requirement, and pick it.

The default data layout contains a 128 bit float, so if we just check the maximum alignment we'll always get at least `16`, even on 32-bit architectures. I could only consider the ones set explicitly by the data layout string, but doesn't that go against the purpose of the defaults?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115888