[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

[Steve Canon via Phabricator via cfe-commits]

scanon added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+        getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+      SRETAttrs.addAlignmentAttr(Align);
     ArgAttrs[IRFunctionArgs.getSRetArgNo()] =
----------------
rjmccall wrote:
> Why only when under-aligned?  Just to avoid churning tests?  I think we should apply this unconditionally.
On mainstream architectures today, there's rarely a benefit to knowing about higher alignment (e.g. MOVUPS is just as fast as MOVAPS if the address is actually aligned), so we won't see significant perf wins from preserving over-alignment in most cases, but it also doesn't cost us anything AFAICT and could deliver wins in some specific cases (e.g. AVX on SNB and IVB, where I think we split underaligned 256b stores into two 128b chunks).

So, yeah, I think we ought to simply unconditionally add the alignment to the sret.


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

https://reviews.llvm.org/D74183