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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to