rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2077
+ getDataLayout().getABITypeAlignment(getTypes().ConvertType(RetTy)))
+ SRETAttrs.addAlignmentAttr(Align);
ArgAttrs[IRFunctionArgs.getSRetArgNo()] =
----------------
dexonsmith wrote:
> 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.
Well, first, I think we're going to end up needing an alignment there in all
cases eventually because of opaque pointer types. But I also think it's just
cleaner and more testable to add the attribute in all cases instead of leaving
it off when the IR type happens to have the right alignment, which can be very
sensitive to the target.
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
