[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

Fri Jul 30 11:01:08 PDT 2021

stevewan added inline comments.

================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1781
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-    FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
----------------
sfertile wrote:
> stevewan wrote:
> > Just noting that the comment says `MaxFieldAlignment - The maximum allowed field alignment. This is set by #pragma pack`, but `__attribute__(packed)` also seems to set it to some large value that is at least as large as the FieldAlign. Maybe edit the comment accordingly for now, and a future follow-on patch if necessary.
> Sorry Steven, not sure what you are asking for. 
> 
> attribute packed will set 'FieldPacked` to true, in which case we ignore the max field alignment  (and why the new comment says, or 1 if packed). IIUC the MaXFieldAlign is set by only by pragma pack and pragma align in the Itanium recored layout builder, and the place where it is set based on attribute packed is int he Microsoft record layout builder and doesn't affect us here.
Sorry Sean, my previous comment was indeed confusing. The old buggy behaviour confused me, and for some reason I thought the code must had went down this if statement when attribute packed is set, hence the previous comment. Now that I take a second look, I realize the problem was actually that we never went into the `if` to fix the FieldAlign. Sorry for the churn.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106900/new/

https://reviews.llvm.org/D106900