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

Hubert Tong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 11 21:57:05 PDT 2020


hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:682
 void CodeGenModule::EmitCXXGlobalDtorFunc() {
   if (CXXGlobalDtors.empty())
     return;
----------------
hubert.reinterpretcast wrote:
> Following from my previous comments on `CXXGlobalDtors`, I am not convinced that `__sterm` functions match the role.
This function is to be renamed.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700
+
+    Fn = CreateGlobalInitOrDestructFunction(
+        FTy, llvm::Twine("__sterm80000000_clang_") + GlobalUniqueModuleId, FI,
----------------
The called function is to be renamed.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1057
+  /// Add a destructor to the C++ global destructor function.
+  void AddCXXDtorEntry(llvm::FunctionCallee DtorFn) {
+    CXXGlobalDtors.emplace_back(DtorFn.getFunctionType(), DtorFn.getCallee(),
----------------
This function is to be renamed.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4458
+
+void XLCXXABI::emitCXXGlobalVarDeclDestructFunc(const VarDecl &D,
+                                                llvm::Function *dtorStub,
----------------
This function is to be renamed.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4470
+  const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();
+  llvm::Function *GlobalVDTermFn = CGM.CreateGlobalInitOrDestructFunction(
+      FTy, FnName.str(), FI, D.getLocation());
----------------
`Term`, being a short form for "termination", the variable name does not match the purpose based on the terminology within the Clang context.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4487
+
+  // Check if the guard variable is zero.
+  CGF.EmitCXXGuardedInitBranch(NeedsDestruct, DestructCallBlock, EndBlock,
----------------
This comment is not helpful. It does not say which action is associated with which state.


================
Comment at: clang/test/CodeGen/aix-constructor-attribute.cpp:2
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
+// RUN: 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s \
----------------
Please split on the `|` with the new line beginning with `FileCheck` indented two spaces in relation to the previous line. Apply throughout.


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

https://reviews.llvm.org/D74166





More information about the cfe-commits mailing list