[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:2414
+    assert(PreferredAlign >= ABIAlign &&
+           "PreferredAlign is at least as large as ABIAlign.");
+    return PreferredAlign;
----------------
Minor nit: s/is/should be/;


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1207
+  if (!Base->Class->isEmpty())
+    HasNonEmptyBaseClass = true;
+
----------------
Just set `HandledFirstNonOverlappingEmptyField` to `true` with a comment before the `if`:
By handling a base class that is not empty, we're handling the "first (inherited) member".


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1799
   if (D->isBitField()) {
+    if (FoundNonOverlappingEmptyFieldToHandle)
+      // For a union, the current field does not represent all "firsts".
----------------
Add a comment before the `if`:
```
    // We're going to handle the "first member" based on
    // `FoundNonOverlappingEmptyFieldToHandle` during the current invocation of
    // this function; record it as handled for future invocations.
```

Given the rationale from the comment, move the subject `if` to immediately after the determination of `FoundNonOverlappingEmptyFieldToHandle`. That way, the setting of `HandledFirstNonOverlappingEmptyField` becomes less complicated to track.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1832
     const ArrayType* ATy = Context.getAsArrayType(D->getType());
     FieldAlign = Context.getTypeAlignInChars(ATy->getElementType());
   } else if (const ReferenceType *RT = D->getType()->getAs<ReferenceType>()) {
----------------
It seems that the code to set `AlignIsRequired` is missing from this path.

```
typedef double Dbl __attribute__((__aligned__(2)));
typedef Dbl DblArr[];

union U {
  DblArr fam;
  char x;
};

U u[2];
extern char x[sizeof(u)];
extern char x[4];
```


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1908
+                   FoundNonOverlappingEmptyFieldToHandle))) {
+    HandledFirstNonOverlappingEmptyField = !IsUnion;
+
----------------
I think the `if` condition above is too complicated as a filter for setting `HandledFirstNonOverlappingEmptyField`. I would prefer if we don't need to set `HandledFirstNonOverlappingEmptyField` here. Please see my other comment about `HandledFirstNonOverlappingEmptyField`.


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

https://reviews.llvm.org/D79719