[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

ebevhan added a comment. I submitted a patch with the changes at D85191 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1778 -std::pair FieldInfo = - Context.getTypeInfoInChars(D->getType()); -EffectiveFieldSize = FieldSize = FieldInfo.first; ebevhan wrote: > It seems that in factoring

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

ebevhan added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1778 -std::pair FieldInfo = - Context.getTypeInfoInChars(D->getType()); -EffectiveFieldSize = FieldSize = FieldInfo.first; It seems that in factoring out this to

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

This revision was automatically updated to reflect the committed changes. Closed by commit rG05ad8e942996: [AIX] Implement AIX special alignment rule about double/long double (authored by Xiangling_L). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 279991. Xiangling_L added a comment. - Simplified the test command line; - Split the `typedef` related tests into two to address the LIT testcase failure on windows platform; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu added a comment. Thanks. No further comments from me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___ cfe-commits mailing list

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 279930. Xiangling_L marked 3 inline comments as done. Xiangling_L added a comment. Add one more testcase; Addressed other comments; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked 13 inline comments as done. Xiangling_L added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2418 + if (!Target->allowsLargerPreferedTypeAlignment()) return ABIAlign; jasonliu wrote: > Should this if statement go above the

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

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()) ` ? When Target does not allow

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added a comment. I've done another pass over the code (but did not get through the tests). I have no comments about the code at this time. My understanding is that @jasonliu will be doing another pass over this patch, so he can approve while I'm away on vacation.

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L added inline comments. Comment at: clang/test/Layout/aix-Wpacked-no-diagnostics.cpp:15 + +int a = sizeof(QQ); hubert.reinterpretcast wrote: > Is there a reason to drop the `FileCheck` checking for the layout? I dropped the `FileCheck` because the

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 277417. Xiangling_L added a comment. Removed unused var; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 Files: clang/include/clang/AST/RecordLayout.h clang/include/clang/Basic/TargetInfo.h

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1244 +if (!Base->Class->isEmpty() && !HandledFirstNonOverlappingEmptyField) { + IsFirstNonEmptyBase = true; + // By handling a base class that is not empty, we're

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 276991. Xiangling_L marked 6 inline comments as done. Xiangling_L added a comment. Set `Handled...` = true for non-AIX power alignment; Addressed other comments; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1246 + +// AIX `power` alignment does not apply the preferred alignment for +// non-union classes if the source of the alignment (the current base in Move the

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 276866. Xiangling_L marked 9 inline comments as done. Xiangling_L added a comment. Fixed a base class related case by adding `IsFirstNonEmpty` flag; Split the `aix-Wpacked.cpp` testcase into two; Addressed other comments; CHANGES SINCE LAST ACTION

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1208 +// "first (inherited) member". +HandledFirstNonOverlappingEmptyField = true; + We need some sort of `IsFirstNonEmptyBase` to record that the current

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1263 - // The maximum field alignment overrides base align. + assert(!IsUnion && "Unions cannot have base classes."); + // AIX `power` alignment does not apply the preferred

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked an inline comment as done. Xiangling_L added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1796 + bool FoundFirstNonOverlappingEmptyFieldToHandle = + DefaultsToAIXPowerAlignment && FieldOffset == CharUnits::Zero() && +

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1796 + bool FoundFirstNonOverlappingEmptyFieldToHandle = + DefaultsToAIXPowerAlignment && FieldOffset == CharUnits::Zero() && + !HandledFirstNonOverlappingEmptyField &&

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 276528. Xiangling_L marked 2 inline comments as done. Xiangling_L added a comment. Fixed a -Wpacked related case and added the case to the tests; Fixed the base class related code issue; Addressed other comments; CHANGES SINCE LAST ACTION

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked 9 inline comments as done. Xiangling_L added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1796 + bool FoundFirstNonOverlappingEmptyFieldToHandle = + DefaultsToAIXPowerAlignment && FieldOffset == CharUnits::Zero() && +

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1254 + // space or zero-extent array. + if (DefaultsToAIXPowerAlignment && !getDataSize().isZero()) { +PreferredBaseAlign = BaseAlign; This needs to check

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1064 setSize(getSize() + PtrWidth); setDataSize(getSize()); } I would suggest setting `HandledFirstNonOverlappingEmptyField` to `true` here with an

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 276143. Xiangling_L marked 14 inline comments as done. Xiangling_L added a comment. Fixed typedef issue on incomplete array field and add a test for it; Added a test for where pack attribute on object also apply on base classes; Addressed other comments;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1225 + Context.getTargetInfo().getTriple().isPS4() || + Context.getTargetInfo().getTriple().isOSAIX())) + ? CharUnits::One()

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1910 + +auto upgradeAlignmentIfQualified = [&](const BuiltinType *BTy) { + if (BTy->getKind() == BuiltinType::Double || "Qualified" is a term of art in the

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2414 +assert(PreferredAlign >= ABIAlign && + "PreferredAlign is at least as large as ABIAlign."); +return PreferredAlign; Minor nit: s/is/should be/;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

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;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2409 +const RecordDecl *RD = RT->getDecl(); +return std::max(ABIAlign, static_cast(toBits( + getASTRecordLayout(RD).PreferredAlignment)));

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 275840. Xiangling_L marked 6 inline comments as done. Xiangling_L added a comment. Fixed the `typedef` related issues; Added more testcases; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 Files:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2409 +const RecordDecl *RD = RT->getDecl(); +return std::max(ABIAlign, static_cast(toBits( + getASTRecordLayout(RD).PreferredAlignment)));

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1885 + Context.getBaseElementType(CTy->getElementType()) + ->getAs()) +if (BTy->getKind() == BuiltinType::Double || Xiangling_L

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 275728. Xiangling_L marked 3 inline comments as done. Xiangling_L added a comment. Fixed -Wpacked warning issue; Fixed EmptySubobjects related offset issue; Fixed zero-extent array in a base class related issue; Addressed other comments; CHANGES SINCE

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked 27 inline comments as done. Xiangling_L added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2424 + (T->isSpecificBuiltinType(BuiltinType::LongDouble) && + Target->supportsAIXPowerAlignment())) // Don't increase the alignment if an

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2424 + (T->isSpecificBuiltinType(BuiltinType::LongDouble) && + Target->supportsAIXPowerAlignment())) // Don't increase the alignment if an alignment attribute was specified on

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1908 // The align if the field is not packed. This is to check if the attribute // was unnecessary (-Wpacked). CharUnits UnpackedFieldAlign = FieldAlign;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1235 + // Do not use AIX special alignment if current base is not the first member or + // the struct is not a union. hubert.reinterpretcast wrote: >

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1877 + CharUnits PreferredAlign = FieldAlign; + if (SupportsAIXPowerAlignment && FieldOffset == CharUnits::Zero() && + (IsUnion || FoundNonOverlappingEmptyField)) {

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ Xiangling_L wrote:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked 5 inline comments as done. Xiangling_L added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1884 + if (const BuiltinType *BTy = + Context.getBaseElementType(CTy->getElementType()) + ->getAs()) I don't think there's a reason

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1778 +if (FoundNonOverlappingEmptyField) + HandledFirstNonOverlappingEmptyField = true; + See my other comment for rationale on why

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1871 + // AIX ABI has this special rule that in aggregates, the first member of + // floating point data type(or aggregate type contains floating point data

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1235 + // Do not use AIX special alignment if current base is not the first member or + // the struct is not a union. hubert.reinterpretcast wrote: > Suggestion:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ Xiangling_L wrote:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked an inline comment as done. Xiangling_L added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2416 - // Double and long long should be naturally aligned if possible. + // Double, long double (only when the target supports AIX power alignment) and + // long long should be

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. 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 %s 2>/dev/null | \ +// RUN: FileCheck %s

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ I am concerned that

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 273708. Xiangling_L marked 2 inline comments as done. Xiangling_L added a comment. Corrected the comments; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 Files:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:75 + // can be different than Alignment in cases where it is beneficial for + // performance or backwards compatibility perserving (e.g. AIX-ABI). + CharUnits PreferredAlignment;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu accepted this revision. jasonliu added a comment. LGTM. I have no further comments on this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___ cfe-commits mailing list

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1881 + if (isAIXLayout(Context) && FieldOffset == CharUnits::Zero() && + (IsUnion || NonOverlappingEmptyFieldFound)) { +FirstNonOverlappingEmptyFieldHandled = true;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L marked 2 inline comments as done. Xiangling_L added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1881 + if (isAIXLayout(Context) && FieldOffset == CharUnits::Zero() && + (IsUnion || NonOverlappingEmptyFieldFound)) { +

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1881 + if (isAIXLayout(Context) && FieldOffset == CharUnits::Zero() && + (IsUnion || NonOverlappingEmptyFieldFound)) { +FirstNonOverlappingEmptyFieldHandled = true;

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 272824. Xiangling_L marked 2 inline comments as done. Xiangling_L added a comment. Adjust the function name; Adjust the comment; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 Files:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2424 + (T->isSpecificBuiltinType(BuiltinType::LongDouble) && + Target->supportsAIXPowerAlignment())) // Don't increase the alignment if an alignment attribute was specified on

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 272741. Xiangling_L marked 24 inline comments as done. Xiangling_L added a comment. Addressed comments; Fixed the ICE problem with array as first member; Add support for Complex type; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2424 + (T->isSpecificBuiltinType(BuiltinType::LongDouble) && + Target->supportsAIXPowerAlignment())) // Don't increase the alignment if an alignment attribute was specified on a

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ There's no testing

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1887 + if (BTy->isFloatingPoint()) { +PreferredAlign = FieldSize; + } I tried `clang -cc1 -triple powerpc-ibm-aix -fsyntax-only` with : ``` struct A

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:660 + /// When there are OverlappingEmptyFields existing in the aggregate, the + /// flag shows if the following first non-overlappingEmptyField has been + /// handled, if any.

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:1389 + /// Whether target supports the special power alignment rules of AIX. + virtual bool supportsAIXPowerAlignment() const { return false; } Minor nit: Use

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:75 + // can be different than Alignment in cases where it is beneficial for + // performance. + CharUnits PreferredAlignment; nit for comment: I don't think it's related to

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 269553. Xiangling_L marked an inline comment as done. Xiangling_L added a comment. Replace the transient status by a local var; Clean up the code; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666 +FirstNonOverlappingEmptyFieldHandled + } FirstNonOverlappingEmptyFieldStatus; + Xiangling_L wrote: > efriedma wrote: > > Instead of specifically tracking whether you've

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 269396. Xiangling_L marked 2 inline comments as done. Xiangling_L added a comment. Simplify the code; Add one more testcase related to [[no_unique_addr]]; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666 +FirstNonOverlappingEmptyFieldHandled + } FirstNonOverlappingEmptyFieldStatus; + efriedma wrote: > Instead of specifically tracking whether you've found an

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666 +FirstNonOverlappingEmptyFieldHandled + } FirstNonOverlappingEmptyFieldStatus; + Instead of specifically tracking whether you've found an OverlappingEmpty field, could

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 268840. Xiangling_L added a comment. Replace `int` with an more self-explanatory enum; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 Files:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:660 + /// with [[no_unique_attr]] Empty CXXRD invovled in the CXXRD. + int IsRealFirstMember; + Please explain here what the different values (-1, 0, 1, 2) mean. Or use an

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L updated this revision to Diff 268624. Xiangling_L marked 11 inline comments as done. Xiangling_L added a comment. Add `PreferredAlignment` and `PreferredNVAlignment` field; Adjust `-fdump-record-layouts` format for AIX; Update the testcase formatting; Repository: rG LLVM Github

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2506 if (!Target->allowsLargerPreferedTypeAlignment()) return ABIAlign; jyknight wrote: > I think from here on down is currently X86-specific, even though it's not > phrased

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jyknight added a comment. In the commit message, you refer to the preferred alignemtn as the "true" alignment, but that's misleading. As discussed on the mailing list thread, it's not true alignment at all. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + jasonliu wrote: > efriedma wrote: > > If we have to keep around extra

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2533 + hasFloatingPointAsFirstMember(RD, MaxFieldAlignment)) +return std::max(ABIAlign, (unsigned)toBits(CharUnits::fromQuantity(8))); +} I guess another thing that

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jasonliu added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + efriedma wrote: > If we have to keep around extra data anyway,

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

efriedma added a comment. This approach seems to reflect the consensus from the mailing list. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + If we

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

Xiangling_L created this revision. Xiangling_L added reviewers: hubert.reinterpretcast, jasonliu, sfertile, cebowleratibm. Xiangling_L added a project: LLVM. Herald added subscribers: cfe-commits, kbarton, nemanjai. Herald added a project: clang. Implement AIX special alignment rule by