Xiangling_L added inline comments.
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:708
+ " based on strong external symbols");
+ GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);
+ }
----------------
jasonliu wrote:
> 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?
Actually `GlobalUniqueModuleId` will not always get “initialized" in
`EmitCXXGlobalInitFunc`. In some cases, only __sterm will be generated for the
var decl.
e.g.
The static local variable won't be inited before main() but when called at the
first time.
```
struct Test4 {
Test4();
~Test4();
};
void f() {
static Test4 staticLocal;
}
```
================
Comment at: clang/test/CodeGen/static-init.cpp:142
+
+// CHECK: ; Function Attrs: noinline nounwind
+// CHECK: define internal void @__finalize__ZZN5test41fEvE11staticLocal() #0 {
----------------
jasonliu wrote:
> Is checking this Attrs necessary?
I missed to delete this line. Thanks for pointing it out.
================
Comment at: clang/test/CodeGen/static-init.cpp:157
+
+// CHECK: define void
@__sinit80000000_clang_1145401da454a7baad10bfe313c46638() #5 {
+// CHECK: entry:
----------------
jasonliu wrote:
> 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?
`dso_local` affects calls to the function in terms of how the compiler will do
the optimization on the call. Since we won't be generating any calls to the
function (the linker arranges for the calls to happen indirectly), marking them
dso_local does not help any. So I think it doesn't really matter if we put
`dso_local` here.
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
