[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

This revision was automatically updated to reflect the committed changes. Closed by commit rC356776: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent (authored by carwil, committed by ). Changed prior to commit: https://reviews.llvm.org/D59094?vs=190798&id=191895#toc Repository: rC C

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

efriedma accepted this revision. efriedma added a reviewer: t.p.northover. efriedma added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/CodeGen/TargetInfo.cpp:6023 + // Variadic functions should always marshal to the base standard. + b

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

carwil updated this revision to Diff 190798. carwil added a comment. Done. I've added an extra parameter over what you might expect as classifyArgumentTypes doesn't seem to consider AAPCS16, whereas classifyReturnTypes does. I've also renamed the variable to make the distinction between it and t

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

efriedma added a comment. Looking more carefully, I guess none of the other places actually distinguish between ARMABIInfo::AAPCS_VFP and ARMABIInfo::AAPCS, so I guess the current change is sufficient given the calling-convention attributes that are likely to be used in practice. Please pull t

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

carwil updated this revision to Diff 190653. carwil marked an inline comment as done. carwil added a comment. I got bit a confused earlier. That does actually seem like correct behaviour (once we're no longer able to treat the struct as a homogeneous aggregate). I've tightened up the conditional

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

carwil added a comment. In D59094#1421893 , @efriedma wrote: > Not sure how to write a testcase off the top of my head... have you tried > homogeneous aggregates with more than two elements? Good catch! Seems like if we surpass "isHomogeneousAggregateSm

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

efriedma added a comment. It looks like every call to getABIKind() is doing the wrong thing; I'm worried that's going to lead to some sort of subtle miscompile without some sort of centralized fix to compute the correct calling convention for every place that checks it. Not sure how to write a

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

carwil created this revision. Herald added subscribers: cfe-commits, jdoerfert, kristof.beyls, javed.absar. Herald added a project: clang. See: https://bugs.llvm.org/show_bug.cgi?id=39982 When considering how to classify homogeneous aggregates as return/argument types, the ABI of the function (s