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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
