[PATCH] D82307: [Alignment][NFC] Use Align for TargetCallingConv::OrigAlign

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a comment.

In D82307#2108220 <https://reviews.llvm.org/D82307#2108220>, @courbet wrote:

> I see several places that never call `setOrigAlign()`, e.g. in GlobalISel CallLowering:
>
>   ISD::ArgFlagsTy Flags = OrigFlags;
>               if (Part == 0) {
>                 Flags.setSplit();
>               } else {
>                 Flags.setOrigAlign(Align(1));
>                 if (Part == NumParts - 1)
>                   Flags.setSplitEnd();
>               }
>               Args[i].Regs.push_back(Reg);
>               Args[i].Flags.push_back(Flags);
>
>
> How did you infer that this change was safe ?


So indeed, running the tests in debug mode revealed a few architectures for which `OrigAlign` was not set.
Namely codegen for `ARM` and `Mips`.

I looked at all the stack traces. They either use the alignment to check on some particular values and make decisions based on them (e.g. which registers to use) or passed down to `AllocateStack`which requires the alignment to be defined <https://github.com/llvm/llvm-project/blob/4c257bb44e7216961cb46d22d4a53844919c8ed0/llvm/lib/Target/Mips/MipsISelLowering.cpp#L2967>.

By switching the default value to `Align(1)` instead of `None`, all the tests are passing.
It is not a proof that the change is safe for sure but we're in CodeGen and alignment should be resolved at this point anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82307