[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/RecordLayoutBuilder.cpp:666
+  /// HasNonEmptyBaseClass - Whether the class has any non-empty class (in the
+  /// sense of (C++11 [meta.unary.prop])) as base.
+  bool HasNonEmptyBaseClass;
----------------
Minor nit: s/as base/as a base/;


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:707
+        HandledFirstNonOverlappingEmptyField(false),
+       	HasNonEmptyBaseClass(false),
+        FirstNearlyEmptyVBase(nullptr) {}
----------------
Minor nit: Please fix the formatting.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1225
+                       Context.getTargetInfo().getTriple().isPS4() ||
+                       Context.getTargetInfo().getTriple().isOSAIX()))
+               ? CharUnits::One()
----------------
Thanks; verified that this is correct with `xlclang++` from IBM XL C/C++ for AIX with:
```
struct A {
  char x;
};
struct B {
  int x;
};
struct __attribute__((__packed__)) C : A, B {} c;
```

Length is 5:
```
[10]    m   0x00000004      .bss     1  extern                    c
[11]    a4  0x00000005       0    0     CM       RW    0    0
```

@Xiangling_L, I suggest adding a case for this to the tests.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1256
+  // space or zero-extent array.
+  if (DefaultsToAIXPowerAlignment && !(getDataSize().isZero() || IsUnion)) {
+    PreferredBaseAlign = BaseAlign;
----------------
Unions cannot have base classes. Please assert `!IsUnion`.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1260
+
+  // The maximum field alignment overrides base align/preferred base align(AIX
+  // only).
----------------
Suggestion:
```
  // The maximum field alignment overrides the base align/(AIX-only) preferred
  // base align.
```


================
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:18
+
+// CHECK-DAG: @_ZN5test11aE = global i32 2, align 4
+
----------------
Instead of checking the value of `a`, the alignment can be checked more directly from the IR for `cc`.


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

https://reviews.llvm.org/D79719