[PATCH] D46013: [ARM] Conform to AAPCS when passing overaligned composites as arguments

Tim Northover via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 22 00:34:53 PDT 2018


t.p.northover added a comment.

I'm fine with the ABI changes, but I'm not very convinced by the "NaturalAlignment" name.

Far from being a privileged alignment kind, it seems to take account of a pretty arbitrary set of modifiers. In fact, I can't help wondering if what's really happened is that ARM has decided to document existing GCC practice as the path of least resistance and scrambled for a name. This would be fine in ARM-specific code but could be quite misleading in the generic ASTContext. Personally I'd go for `ARMNaturalAlignment` and stop pretending it's something anyone else needs to care about (at least until some other ABI comes along that does).



================
Comment at: lib/CodeGen/TargetInfo.cpp:5055
+      Alignment = getContext().getTypeNaturalAlign(Ty);
+      Alignment = std::min(std::max(Alignment, 64u), 128u);
+    } else {
----------------
I think the max/min logic is more confusing here than the alternative:

    Alignment = Alignment < 128 ? 64 : 128;


https://reviews.llvm.org/D46013





More information about the cfe-commits mailing list