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