[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

This revision was automatically updated to reflect the committed changes. Closed by commit rG8bee52bdb54a: [AIX][Frontend] C++ ABI customizations for AIX boilerplate (authored by Xiangling_L). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74015/new/

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

jasonliu added a comment. Just kindly reminder that you might want to update the Summary in the commit message to reflect the new name we picked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74015/new/ https://reviews.llvm.org/D74015

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

sfertile accepted this revision. sfertile 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/D74015/new/ https://reviews.llvm.org/D74015

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L updated this revision to Diff 245844. Xiangling_L marked an inline comment as done. Xiangling_L added a comment. Update the comment about ABI description; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74015/new/

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. In D74015#1882933 , @sfertile wrote: > Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside > my wheelhouse though, so I can't confirm things like the tail padding rules > are correct for AIX.

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

sfertile added a comment. Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside my wheelhouse though, so I can't confirm things like the tail padding rules are correct for AIX. Because of that I'm not comfortable being the one to accept the patch. CHANGES SINCE LAST

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L added a comment. In D74015#1880847 , @cebowleratibm wrote: > From my perspective, the only issue holding this up is settling on the name. > I'd like to hammer that out and get this committed. It looks everyone agrees on `XL` so far. As

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. From my perspective, the only issue holding this up is settling on the name. I'd like to hammer that out and get this committed. Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +/// - static initialization is adjusted to use

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L marked an inline comment as done. Xiangling_L added a comment. ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74015/new/ https://reviews.llvm.org/D74015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L marked 3 inline comments as done. Xiangling_L added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLCXXABI final : public ItaniumCXXABI { +public: sfertile wrote: > Xiangling_L wrote: > > sfertile wrote: > > > Here

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

sfertile added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLCXXABI final : public ItaniumCXXABI { +public: Xiangling_L wrote: > sfertile wrote: > > Here would be a good place to add a comment to indicate that XL has several >

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L marked 3 inline comments as done. Xiangling_L added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLCXXABI final : public ItaniumCXXABI { +public: sfertile wrote: > Here would be a good place to add a comment to

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

sfertile added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLCXXABI final : public ItaniumCXXABI { +public: Here would be a good place to add a comment to indicate that XL has several C++ ABIs, but this represents the one used

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L updated this revision to Diff 244476. Xiangling_L marked 4 inline comments as done. Xiangling_L added a comment. Adjust ABI name to `XL`; Simplify the testcase; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74015/new/ https://reviews.llvm.org/D74015 Files:

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. Looks fine, but we need to settle on the name for the ABI. My preference would be "XLC++11", or perhaps "XLCXX11" (I propose the latter because of the common reference CXXABI.) Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +///

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

sfertile added inline comments. Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +/// - static initialization is adjusted to use sinit and sterm functions; +XL_Clang, + Xiangling_L wrote: > daltenty wrote: > > Why the underscore in the name?

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L marked 4 inline comments as done. Xiangling_L added inline comments. Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +/// - static initialization is adjusted to use sinit and sterm functions; +XL_Clang, + daltenty wrote: > Why the

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

stevewan added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLClangCXXABI final : public ItaniumCXXABI { +public: The class name here is inconsistent with how our ABI kind was called previously, as David pointed out. Maybe

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

daltenty added inline comments. Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +/// - static initialization is adjusted to use sinit and sterm functions; +XL_Clang, + Why the underscore in the name? This is a bit inconsistent with both the

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

Xiangling_L created this revision. Xiangling_L added reviewers: hubert.reinterpretcast, cebowleratibm, yusra.syeda, sfertile, jasonliu, xingxue. Xiangling_L added a project: LLVM. Herald added a project: clang. Herald added a subscriber: cfe-commits. This PR enables **XL_Clang** C++ ABI in