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

[Chris Bowler via Phabricator via cfe-commits]

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
+    ///   - static initialization is adjusted to use sinit and sterm functions;
+    XL_Clang,
+
----------------
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)


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4428
+                                   llvm::Constant *addr) {
+  llvm::report_fatal_error("Static initialization has not been fully"
+                           " implemented on XL_Clang ABI yet.");
----------------
Omit "fully".  Fix the name XL_Clang when we've settled on a name.


================
Comment at: clang/test/CodeGen/static-init.cpp:9
+public:
+    test(int c) {a = c;}
+    ~test() {a = 0;}
----------------
Nit: formatting.  

The test is fine but I'd usually write this test even more briefly:
struct S { ~S(); } s;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74015