jasonliu added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:2418
+
if (!Target->allowsLargerPreferedTypeAlignment())
return ABIAlign;
----------------
Should this if statement go above the `if (const auto *RT =
T->getAs<RecordType>()) ` ?
When Target does not allow larger prefered type alignment, then we should
return ABIAlign immediately without going through the RecordType query?
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1234
+
+ bool DefaultsToAIXPowerAlignment =
+ Context.getTargetInfo().defaultsToAIXPowerAlignment();
----------------
Add const?
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1801
+
+ bool DefaultsToAIXPowerAlignment =
+ Context.getTargetInfo().defaultsToAIXPowerAlignment();
----------------
const ?
================
Comment at: clang/lib/Basic/Targets/PPC.h:425
+ } else if (Triple.isOSAIX()) {
+ SuitableAlign = 64;
+ LongDoubleWidth = 64;
----------------
SuitableAlign is set in line 412 as well. Please consider combining the two
`if` statements if grouping things together makes code easier to parse.
================
Comment at: clang/test/Layout/aix-double-struct-member.cpp:2
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \
+// RUN: -fdump-record-layouts -fsyntax-only -x c++ %s | \
+// RUN: FileCheck %s
----------------
You are not using ` < %s` here. So `-x c++` is redundant?
================
Comment at: clang/test/Layout/aix-double-struct-member.cpp:305
+struct A { double x; };
+struct B : A {} b;
+
----------------
`b` is not necessary when you take the `sizeof` of B below?
================
Comment at: clang/test/Layout/aix-no-unique-address-with-double.cpp:138
+// CHECK-NEXT: | nvsize=8, nvalign=4, preferrednvalign=4]
+
+int a = sizeof(Empty);
----------------
I think this case is interesting and may worth adding too:
```
struct F {
[[no_unique_address]] Empty emp, emp2;
double d;
};
```
================
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:12
+ double x;
+} c;
+typedef struct C __attribute__((__aligned__(2))) CC;
----------------
remove `c` ?
================
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:25
+ Dbl x;
+} a;
+
----------------
remove `a`?
================
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:43
+ char x;
+} u;
+
----------------
remove `u`?
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
