efriedma added inline comments.
================ Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + ---------------- jasonliu wrote: > efriedma wrote: > > If we have to keep around extra data anyway, probably better to add a field > > for the preferred alignment, so we can keep the preferred alignment > > handling together. > We could get the MaxFieldAlignment in the same way > ItaniumRecordLayoutBuilder::InitializeLayout get. Do we need to keep an extra > data? The idea here is that we want to make the division of work more clear: RecordLayoutBuilder actually does the relevant computation, and ASTContext just returns the result. ================ Comment at: clang/lib/AST/ASTContext.cpp:2533 + hasFloatingPointAsFirstMember(RD, MaxFieldAlignment)) + return std::max(ABIAlign, (unsigned)toBits(CharUnits::fromQuantity(8))); + } ---------------- jasonliu wrote: > I guess another thing that worth considering is how we support "pragma > align(nature)" in the future. It requires us to get a normal alignment for > AIX which means double will be aligned 8 byte in the structure. > Right now, we always set the double to be aligned 4 byte in PPC.h, but it's > not necessary the case if that pragma is in effect. How double should be > aligned in a structure is really position dependent (is the structure comes > after `pragma align(nature)` or `pragma align(power)`?) I'm not very clear > on how we would handle that with the current architect. We convert pragmas like that into attributes in the AST. See Sema::AddAlignmentAttributesForRecord. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits