[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:824
       std::tie(CalleeTy, Callee, Arg) = DtorsAndObjects[e - i - 1];
-      llvm::CallInst *CI = Builder.CreateCall(CalleeTy, Callee, Arg);
+      llvm::CallInst *CI = (Arg == nullptr)
+                               ? Builder.CreateCall(CalleeTy, Callee)
----------------
Please assert that `Arg == nullptr` iff we're dealing with sinit/sterm.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+    CXXGlobalDtors.emplace_back(DtorFn.getFunctionType(), DtorFn.getCallee(),
+                                nullptr);
----------------
Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > The description of `CXXGlobalDtors` is
> > > Global destructor functions and arguments that need to run on termination.
> > 
> Currently, only Apple Kernal extension will use `AddCXXDtorEntry` to add variable destructor to CXXGlobalDtors.
> An example is:
> 
> ```
> $cat test.cpp
> class test {
>    int a;
> public:
>     test(int c) {a = c;}
>     ~test() {a = 0;}
> } t(1);
> 
> 
> $clang --driver-mode=g++ -target x86_64-apple-darwin10 -fapple-kext -flto -S -o - test.cpp
> 
> ...
> define internal void @_GLOBAL__D_a() #0 {
> entry:
>   call void @_ZN4testD1Ev(%class.test* @t)       #~test() {a = 0;}
>   ret void
> }
> ```
> 
> Since the usage as you said below does not match with our sterm finalizer function `__cxx_global_var_destruct`, I am thinking we can either modify the name and the description of `CXXGlobalDtors` to make it also fit AIX or we can define a brand new `CXXStermFinalizers` vector and also related facility to `GenerateCXXStermFinalizerFunc` instead. 
> 
> An example of modification I have in mind is:
> `CXXGlobalDtorsOrStermFinalizers`
> 
> 
> > Global destructor functions and arguments that need to run on termination;
> > When UseSinitAndSterm is set, it contains sterm finalizer functions instead that need to run on unloading a shared library.
> 
> 
> 
> I would prefer the modification way to save a lot effort. Any thoughts on it?
I'm fine with renaming but it should include renaming the associated fuctions.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4447
+
+  // Create __dtor function for the var decl.
+  llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr);
----------------
Xiangling_L wrote:
> hubert.reinterpretcast wrote:
> > We should probably report_fatal_error if `CXAAtExit` (the option) is true before this line. This would imply driver changes to default AIX to using `-fno-use-cxa-atexit`. An associated test file is https://github.com/llvm/llvm-project/blame/master/clang/test/Driver/cxa-atexit.cpp.
> Maybe we should add `-fno-use-cxa-atexit` to driver in a follow-up patch?
I'm okay with that.


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

https://reviews.llvm.org/D74166