dexonsmith added inline comments.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+ getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+ SRETAttrs.addAlignmentAttr(Align);
ArgAttrs[IRFunctionArgs.getSRetArgNo()] =
----------------
scanon wrote:
> 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.
@rjmccall, are you seeing a reason to add the attribute when the implicit one
is correct (neither over-aligned nor under-aligned)? If so, it seems to me
like the added noise would make the IR harder to read.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74183/new/
https://reviews.llvm.org/D74183
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
