[PATCH] D125095: [Clang][AIX] Add .ref in frontend for AIX XCOFF to support `-bcdtors:csect` linker option

[Ting Wang via Phabricator via cfe-commits]

tingwang marked an inline comment as done.
tingwang added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:688
+        updateAssociatedFunc(VFInitTermAssoc, LocalCXXGlobalInits, GetElem, Fn);
+        updateAssociatedFunc(FFDtorTermAssoc, LocalCXXGlobalInits, GetElem, Fn);
+      }
----------------
shchenz wrote:
> `FFDtorTermAssoc` should store the mapping between dtor and term functions? So why we need to update this container when we generate wrapper function for init function? I think in the init function there should only be ctors related functions?
> 
> And why we don't need to update for `VarsWithInitTerm`, in that container there should be some static variables reply on the wrapper init function?
> `FFDtorTermAssoc` should store the mapping between dtor and term functions? So why we need to update this container when we generate wrapper function for init function? I think in the init function there should only be ctors related functions?

Thank you for pointing out! This is redundant.

> And why we don't need to update for `VarsWithInitTerm`, in that container there should be some static variables reply on the wrapper init function?

VarsWithInitTerm keeps track of mapping between variables in clang (Decl*) and the corresponding data structure in llvm (Constant *). To me it's stable, and not like functions which could be wrapped in new functions.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4799
+    if (getTriple().isOSAIX())
+      addVarWithInitTerm(D, GV);
+  }
----------------
shchenz wrote:
> Why do we need to add mapping between a variable and its address? We already map the global and its init function in above `EmitCXXGlobalVarDeclInitFunc`?
It seems to me that clang most of the time operates on Decl*. However to generate metadata, we refer to llvm::Constant*. I did not find how to get llvm::Constant* from Decl* in clang, so I'm tracking that information. I will check again to see if there is any official way to do that but I'm not aware of.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:465
+  /// between dtor and term functions.
+  llvm::SmallVector<std::pair<const Decl *, llvm::Constant *>, 8>
+      VFInitTermAssoc;
----------------
shchenz wrote:
> Is there any reason why we need `vector` here instead of `map`? Can you give an example that shows one global variable will be connected with more than one init functions?
One variable can have two functions associated: one init and one term, thus used vector for VFInitTermAssoc. Also it is better to use variable as key for the benefit of the inner for loop inside AddMeta.

Dtor-to-Term (FFDtorTermAssoc) could use map, however it shares similar update logic as VFInitTermAssoc (for example the code snippet AddMeta in CodeGenModule::genAssocMeta() is used on both data structure), so I prefer to use vector for both of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125095