efriedma added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4539
CharUnits CCAlign = getParamTypeAlignment(Ty);
CharUnits TyAlign = getContext().getTypeAlignInChars(Ty);
----------------
jyknight wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > Question:
> > > It looks like getNaturalAlignIndirect and getTypeAlignInChars here are
> > > all returning ABI alignment.
> > > But according to the comments, we should use a preferred alignment when
> > > it's a complete object. Isn't this complete object? Or I'm missing
> > > something?
> > @jyknight Could you shine a light on this? Personally, I would agree that
> > we have complete objects here, so preferred alignment should be used. And
> > if that is true, changes should be applied on all other target within this
> > file?
> This code specifies that aggregate parameters are passed on the stack aligned
> to CCAlign (4 or 8), even when the aggregate _requires_ higher alignment.
> E.g. `struct X { int a, b; } __attribute__((aligned(8)));` would only be
> passed on the stack with 4-byte alignment for powerpc-aix. The callee needs
> the object to have the correct 8-byte alignment, so it must copy the
> parameter to an 8-byte aligned temporary object (this is what "Realign" asks
> for).
>
> We don't want that overhead for optional (preferred) alignment. It wouldn't
> be wrong, but would degrade performance and isn't needed. So, no, this
> shouldn't use preferred alignment.
Are you sure we can get away with skipping the realignment? Consider something
like the following:
```
struct A { double x; };
void f(int x, struct A a) { _Static_assert(_Alignof(a) == 8); }
```
_Alignof says the alignment is 8, but the IR says "align 4". Is that safe?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86790/new/
https://reviews.llvm.org/D86790
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
