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