Xiangling_L added inline comments.
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+ if (CXXGlobalInits.empty())
+ return;
----------------
hubert.reinterpretcast wrote:
> Can this part be committed in a separate patch? It does not appear to have
> dependencies on other parts of this patch and has the appearance of being a
> possible change for other platforms.
Sure. I will create a NFC patch for this part and try to commit it first. But
so far, I think I can keep this part in this patch just for review purpose to
make the patch look nicer?
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:704
AddGlobalDtor(Fn);
+ CXXGlobalDtors.clear();
}
----------------
hubert.reinterpretcast wrote:
> If this is another drive-by fix, can we put it in a separate patch?
Sure, I will put it in the above mentioned clean-up NFC patch as well.
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+ Fn = CreateGlobalInitOrDestructFunction(
+ FTy, llvm::Twine("__sterm80000000_clang_") + GlobalUniqueModuleId, FI,
----------------
hubert.reinterpretcast wrote:
> The called function is to be renamed.
Any suggestions here about how to represent the functionality of `__sterm` and
`_GLOBAL__D_a` into one word? Cannot think of a good name...
Actually I am wondering do we need to rename the `Destruct` part? The `__sterm`
and `_GLOBAL__D_a` do destruct the object by calling sterm finalizers and
object destructors?
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
llvm::Function *Fn,
----------------
hubert.reinterpretcast wrote:
> This function is to be renamed.
I will try `GenerateCXXGlobalDestructFunc` based on the thoughts as I also
mentioned elsewhere that the `__sterm` and `_GLOBAL__D_` function do destruct
the object by calling sterm finalizers and object destructors.
Any suggestions or concerns about this renaming? Or do we really need to rename
this one?
================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+ // Create a variable destruction function.
+ const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();
----------------
hubert.reinterpretcast wrote:
> Suggestion:
> Create the finalization action associated with a variable.
I don't get your point, can you elaborate on this a little bit?
================
Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN: FileCheck %s
struct test {
----------------
jasonliu wrote:
> Looks like the non-inline comments are easier to get ignored and missed, so I
> will copy paste the non-inlined comment I previously had:
> ```
> -fregister_global_dtors_with_atexit does not seem to work properly in current
> implementation.
> We should consider somehow disabling/report_fatal_error it instead of letting
> it generate invalid code on AIX.
> ```
Thanks for doing so!
The semantic of `global_dtors` here on AIX is `__sterm` function, which we are
never gonna register with `__atexit`. I am thinking we can disable it in a
follow-up driver patch with the handling of `-fno-use-cxa-atexit`.
Repository:
rG LLVM Github Monorepo
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
