efriedma added inline comments.
================
Comment at: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:370
+ const auto StorageAlignment = getAlignment(StorageType);
+ if (LayoutSize % StorageAlignment || Layout.getDataSize() % StorageAlignment)
Packed = true;
----------------
dzhidzhoev wrote:
> efriedma wrote:
> > Should this be `if (Layout.getSize() % StorageAlignment ||
> > Layout.getDataSize() % StorageAlignment)`? The dependency on
> > isNoUniqueAddress is a bit confusing.
> For base class subobjects, base class DataSize is used to compute the whole
> object size in RecordLayoutBuilder.cpp, and base subobject LLVM type (with
> ".base" suffix) is computed in addition to standard layout LLVM type in
> CGRecordLayoutBuilder.cpp.
>
> Currently, both base subobject and standard layout LLVM types of the same
> class are supposed to agree on packedness, as stated in bb51300970b7. Thus,
> if one is packed, both are marked as packed, as done in
> CGRecordLowering::determinePacked.
>
> For data members of class/struct type, declared with [[no_unique_address]]
> attribute, DataSize is used to compute the whole object size in
> RecordLayoutBuilder.cpp, but standard layout LLVM type is used to represent
> the field in the whole class's LLVM type.
> This patch proposes to use the base subobject LLVM type instead of the
> default LLVM type of a class to lay out a no_unique_address member of this
> class type.
>
> Nextly, the patch proposes to create an unpadded LLVM type for unions, in
> addition to the default LLVM type. It is used to lay out the union members
> having no_unique_address attribute. Existing code for creating base subobject
> layout of classes are reused for unions, therefore, packedness of
> potentially-overlapping union LLVM type and of default union LLVM types must
> agree as well as for classes.
That doesn't really get at what I was asking.
`LayoutSize % StorageAlignment || Layout.getDataSize() % StorageAlignment` is
basically equivalent to `isNoUniqueAddress ? Layout.getDataSize() %
StorageAlignment != 0 : (Layout.getSize() % StorageAlignment ||
Layout.getDataSize() % StorageAlignment)`. From my understanding, you want
isNoUniqueAddress=true to agree with isNoUniqueAddress=false on whether the
result is packed. So why are the checks different?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139741/new/
https://reviews.llvm.org/D139741
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
