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

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm added a comment.

In D74015#1882933 <https://reviews.llvm.org/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. Because of that I'm not comfortable being the one to accept the patch.


I checked the IBM xlclang source and can confirm the implementation of getTailPaddingUseRules is consistent with the proposed patch.  Sean, does that address your concern?

The only change I would like to see is an update to the "XL" enumerator comment to be more descriptive of what the "XL" ABI is.



================
Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+    ///   - static initialization is adjusted to use sinit and sterm functions;
+    XL_Clang,
+
----------------
cebowleratibm wrote:
> cebowleratibm wrote:
> > sfertile wrote:
> > > Xiangling_L wrote:
> > > > daltenty wrote:
> > > > > Why the underscore in the name? This is a bit inconsistent with both the LLVM naming convention here and the name as it appears in other sources.
> > > > There are various AIX ABI. So to distinguish the one we are implementing, we choose `XL` and `Clang` as two parts of the abi name. 
> > > > `XL` - not g++;
> > > > `Clang` - it's a  ABI implemented in Clang;
> > > > 
> > > > And also `XLClang` is misleading because it represents our AIX XL C/C++ compiler itself externally.
> > > So do we need the 'Clang' part in the name? For example the ABI below is not `Microsoft_Clang`. Or is the `_Clang` differentiating between multiple XL ABIs?
> > I suspect the concern is that "XL" ABI is ambiguious between legacy xlC and xlclang++.  The two differ at the C++11 language level so perhaps it makes sense to have "XLC++11"?  (and theoretically just "XL" if we ever decide xlC)
> Another suggestion: "IBMXL" or "IBMXLC++11"
To summarize some off phabricator discussion: we reached consensus on "XL".  It was not felt that "IBM" needs to be in the name and that a comment by the enumerator definition is sufficient.

Note that "XL" refers to ABI compatibiliity with the AIX xlclang compiler and not xlC.  If we need xlC compatibility that would be yet another ABI to add at another time.

The comment should be updated to provide more background on what the "XL" ABI is.


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

https://reviews.llvm.org/D74015