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