[PATCH] D88659: [FE]Split SuitableAlign into two parts

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Hm, to start with, the current state of this confuses me.

In GCC, the preprocessor macro `__BIGGEST_ALIGNMENT__` was supposed to expose the alignment used by `__attribute__((aligned))` (no arg specified), as well the alignment used for alloca. However, this is no longer the case on x86: `BIGGEST_ALIGNMENT` is 512bits with avx-512 enabled, 256bits with avx enabled, and otherwise 128bits. Alloca follows this too. But, `__attribute__((aligned))` was fixed at 128bit alignment, regardless of AVX being enabled, in order to not break ABI compatibility with structs using that. On other architectures, the 3 values seem to be always the same.

In clang, we similarly have (before this patch) both DefaultAlignForAttributeAligned (used for ``attribute((aligned))`), and SuitableAlign (used for the predefined `__BIGGEST_ALIGNMENT__` and alignment for alloca). But these values are different on very many architectures...which I think is probably wrong. Furthermore, SuitableAlign isn't being adjusted to be suitable for vectors, like it is in gcc, which _also_ seems wrong. Looks like there's actually an earlier patch to fix that which was never merged: https://reviews.llvm.org/D39313

So, anyways -- back to this patch: On AIX PPC, you want alloca to align to 128bits, `__attribute__((aligned))` to align to 128bits (aka 8 bytes), but `__BIGGEST_ALIGNMENT__` to only be 4?

That seems pretty weird, and probably wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88659