[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:2409
+    const RecordDecl *RD = RT->getDecl();
+    return std::max(ABIAlign, static_cast<unsigned>(toBits(
+                                  getASTRecordLayout(RD).PreferredAlignment)));
----------------
Please add a comment regarding the situations where the `ABIAlign` value is greater than the `PreferredAlignment` value. It may be appropriate to assert that, absent those cases, the `PreferredAlignment` value is at least that of `ABIAlign`.


================
Comment at: clang/lib/AST/ASTContext.cpp:2409
+    const RecordDecl *RD = RT->getDecl();
+    return std::max(ABIAlign, static_cast<unsigned>(toBits(
+                                  getASTRecordLayout(RD).PreferredAlignment)));
----------------
hubert.reinterpretcast wrote:
> Please add a comment regarding the situations where the `ABIAlign` value is greater than the `PreferredAlignment` value. It may be appropriate to assert that, absent those cases, the `PreferredAlignment` value is at least that of `ABIAlign`.
It does not appear that the maximum of the two values is the correct answer:
```
struct C {
  double x;
} c;
typedef struct C __attribute__((__aligned__(2))) CC;

CC cc;
extern char x[__alignof__(cc)];
extern char x[2]; // this is okay with IBM XL C/C++
```


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1888
+            BTy->getKind() == BuiltinType::LongDouble) {
+          PreferredAlign = CharUnits::fromQuantity(8);
+        }
----------------
hubert.reinterpretcast wrote:
> I believe an assertion that `PreferredAlign` was 4 would be appropriate.
It seems that overriding the value should only be done after additional checks:
```
typedef double __attribute__((__aligned__(2))) Dbl;
struct A {
  Dbl x;
} a;
extern char x[__alignof__(a)];
extern char x[2]; // this is okay with IBM XL C/C++
```

I am getting concerned that the logic here overlaps quite a bit with `getPreferredTypeAlign` and refactoring to make the code here more common with `getPreferredTypeAlign` is necessary.


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

https://reviews.llvm.org/D79719