tmatheson added a comment. Looks sensible but I don't fully understand the context of the change. Please could you explain more what is wrong with the current behaviour, and which parts of the AAPCS you are referring to.
================ 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 = ---------------- Does the similar code added in D100853 need updated too? ================ 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( ---------------- 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. ================ Comment at: clang/test/CodeGen/aarch64-ABI-align-packed.cpp:1 +// RUN: %clang_cc1 -triple aarch64-arm-none-eabi \ +// RUN: -target-feature +v8a \ ---------------- Does this need the `arm` vendor in the triple? ================ Comment at: clang/test/CodeGen/aarch64-ABI-align-packed.cpp:1 +// RUN: %clang_cc1 -triple aarch64-arm-none-eabi \ +// RUN: -target-feature +v8a \ ---------------- tmatheson wrote: > Does this need the `arm` vendor in the triple? Please add a brief comment explaining what this is testing. 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