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