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

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+    Fn = CreateGlobalInitOrDestructFunction(
+        FTy, llvm::Twine("__sterm80000000_clang_") + GlobalUniqueModuleId, FI,
----------------
Xiangling_L wrote:
> 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?
Being clear in the naming of the function helps to signal its purpose to future maintainers. The sterm finalizers are unlikely to be understood from `Destruct` (it implies plain destruction a bit too much). Maybe "cleanup"?


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807
 
 void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
     llvm::Function *Fn,
----------------
Xiangling_L wrote:
> 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?
I think "cleanup" works here too.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:639
+  if (CXXGlobalInits.empty())
+    return;
 
----------------
Xiangling_L wrote:
> 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?
Sure; you'd need the new patch to exist before rebasing on it anyway. We can leave the rebasing to the commit or later in the review.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4467
+
+  // Create a variable destruction function.
+  const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();
----------------
Xiangling_L wrote:
> 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?
This is my suggestion for the comment (to replace "Create a variable destruction function").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166