[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs and fields

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu marked 2 inline comments as done.
JiruiWu added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:5811
+    // For alignment adjusted HFAs, cap the argument alignment to 16, otherwise
+    // set it to 8 according to the AAPCS64 document.
     unsigned Align =
----------------
tmatheson wrote:
> Does the similar code added in D100853 need updated too?
No, because this patch is on AArch64 and https://reviews.llvm.org/D100853 is on AArch32. The alignment is capped to 16 in AArch64 and capped to 8 in AArch32.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:5814
         getContext().getTypeUnadjustedAlignInChars(Ty).getQuantity();
-    unsigned BaseAlign = getContext().getTypeAlignInChars(Base).getQuantity();
-    Align = (Align > BaseAlign && Align >= 16) ? 16 : 0;
+    Align = (Align >= 16) ? 16 : 8;
     return ABIArgInfo::getDirect(
----------------
tmatheson wrote:
> Does this code definitely only apply when the ABI is AAPCS64, or should there be a check for that somewhere here? I can't tell whether the `if` on line 5806 is sufficient.
The code on line 5814 only applies when the ABI is AAPCS64 because it is in the `if` block that starts on line 5805 and ends on line 5818. As a result, the `if` on line 5806 is sufficient.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146242