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

Fri Jun 16 03:20:13 PDT 2023

tmatheson requested changes to this revision.
tmatheson added a comment.
This revision now requires changes to proceed.

I think the current patch is wrong for a couple of reasons.

Firstly the data types being tested, e.g. `struct S { int8x16_t m; }` etc, are not just composite types, but HVAs:
https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#homogeneous-aggregates

> A Homogeneous Aggregate is a composite type where all of the Fundamental Data Types of the members that compose the type are the same. The test for homogeneity is applied after data layout is completed and without regard to access control or other source language restrictions. Note that for short-vector types the fundamental types are 64-bit vector and 128-bit vector; the type of the elements in the short vector does not form part of the test for homogeneity.

So these are HVAs with Fundamental Data Type of 128-bit vector. This explains why the alignment changes get applied, because they are scoped to `if(isHomogeneousAggregate)` which we would not expect to apply for normal composite types.

Since these are HVAs the relevant AAPCS64 rules are different. Specifically 
https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#parameter-passing

> B.3 If the argument type is an HFA or an HVA, then the argument is used unmodified.

would be the "first matching rule" and B6 <https://reviews.llvm.org/B6> (the "alignment of the copy is either 8 or 16" rule) would not be applied.

If anyone can confirm or correct my reading of the above that would be appreciated. The rules are so spread out it's hard to be confident that I've taken everything into account.

I haven't checked if the current alignment for these HVA types is correct yet.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146242