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

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 16 13:44:11 PST 2021


lebedev.ri added a comment.

In D115888#3198672 <https://reviews.llvm.org/D115888#3198672>, @jhuber6 wrote:

> 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,

For which target/architecture? What happens on other target/architectures?

> 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



More information about the llvm-commits mailing list