[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

In D146242#4427966 <https://reviews.llvm.org/D146242#4427966>, @tmatheson wrote:

> In D146242#4427707 <https://reviews.llvm.org/D146242#4427707>, @chill wrote:
>
>> I was just thinking to LGTM it :)
>>
>> IMHO, the alignment adjustment  happens because of C.4 (B.3 indeed leave the HFA/HVA unmodified).
>>
>>> C.4	If the argument is an HFA, an HVA, a Quad-precision Floating-point or short vector type then the NSAA is rounded up to the next multiple of 8 if its natural alignment is ≤ 8 or the next multiple of 16 if its natural alignment is ≥ 16.
>
> I think that C2 would be hit first, suggesting it should be allocated a SIMD register and alignment should be irrelevant, assuming sufficient registers:

Sure, but this is not relevant. We should output a correct `alignstack` attribute if in the end it turns out the argument needs to be allocated in memory. No harm done if we output the attribute, but the
argument ends up in registers.

>> C.2 If the argument is an HFA or an HVA and there are sufficient unallocated SIMD and Floating-point registers (NSRN + number of members ≤ 8), then the argument is allocated to SIMD and Floating-point registers (with one register per member of the HFA or HVA). The NSRN is incremented by the number of registers used. The argument has now been allocated.
>
> If not enough registers, the size also needs rounded up:
>
>> C.3 If the argument is an HFA or an HVA then the NSRN is set to 8 and the size of the argument is rounded up to the nearest multiple of 8 bytes.

I believe that is handled in the backend, by allocating arguments to at least 8-byte aligned stack slots, e.g. here https://github.com/llvm/llvm-project/blob/459f495f49a197a042890e1daa0a98cbae892d2b/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp#L200

> After that C4 would indeed be hit. However C4 differs from B6 <https://reviews.llvm.org/B6>, in that C4 rounds up to the nearest multiple of 8 or 16 (which is not what the patch currently does) whereas B6 <https://reviews.llvm.org/B6> restricts it to either 6 or 16 (which this what this patch does, but shouldn't apply to HVAs).

But there isn't any other power of two between 8 and 16.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146242