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

Reply via email to