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

Jason Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 17 08:03:52 PDT 2020


jasonliu added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:708
+            " based on strong external symbols");
+      GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);
+    }
----------------
Correct me if I'm wrong...
`GlobalUniqueModuleId` will always get “initialized" in `EmitCXXGlobalInitFunc`. And `EmitCXXGlobalInitFunc` will always run before `EmitCXXGlobalCleanUpFunc` in `CodeGenModule::Release()`. So in theory, we do not need to initialize it again in here. 
If we could not `assert (!GlobalUniqueModuleId.empty())` here, that tells me in `EmitCXXGlobalInitFunc` we effectively get an empty string after `GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);`. So something is not right?


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:471
 
-  /// Global destructor functions and arguments that need to run on termination.
+  /// Global destructor functions and arguments that need to run on termination;
+  /// When UseSinitAndSterm is set, it instead contains sterm finalizer
----------------
Word after `;` should not be Capitalize. We should use small case, or change this to `.`.
I would prefer changing it to `.`.


================
Comment at: clang/test/CodeGen/static-init.cpp:142
+
+// CHECK: ; Function Attrs: noinline nounwind
+// CHECK: define internal void @__finalize__ZZN5test41fEvE11staticLocal() #0 {
----------------
Is checking this Attrs necessary?


================
Comment at: clang/test/CodeGen/static-init.cpp:157
+
+// CHECK: define void @__sinit80000000_clang_1145401da454a7baad10bfe313c46638() #5 {
+// CHECK: entry:
----------------
I think we used to have dso_local marked on sinit and sterm function in previous diff. Now they are gone. What's the reason for removing them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74166





More information about the cfe-commits mailing list