jasonliu added inline comments.

Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4609
+    // their own llvm.global_dtors entry.
+    CGM.AddCXXStermFinalizerToGlobalDtor(StermFinalizer, 65535);
+  else
Handling template instantiation seems fairly orthogonal to "moving naming logic 
from frontend to backend". Could we put it in a separate patch (which could be 
a child of this one)? 

Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:460
+  virtual void emitXXStructor(const DataLayout &DL, const int Priority,
+                              const unsigned index, Constant *CV, bool isCtor) 
+    llvm_unreachable("Emit CXXStructor with priority is target-specific.");
Remove `const` from value type (e.g. Priority and index), as there is no real 
need for it.

Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2165
+      emitXXStructor(DL, S.Priority, index, S.Func, isCtor);
+      ++index;
+      continue;
Maybe a naive quesiton, in what situation would we have name collision and need 
`index` as suffix to differentiate?
Could we just report_fatal_error in those situation?

Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1848
+    // beforing emitting them.
+    if (isSpecialLLVMGlobalArrayForStaticInit(&G)) {
+      if (GlobalUniqueModuleId.empty()) {
This would have conflict with D84363. You might want to rebase it later. 

Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1935
+  Function *Func = cast<Function>(CV);
+  Func->setLinkage(GlobalValue::ExternalLinkage);
+  Func->setName((isCtor ? llvm::Twine("__sinit") : llvm::Twine("__sterm")) +
Changing Function name and linkage underneath looks a bit scary. People would 
have a hard time to map IR to final assembly that gets produced. Have you 
thought about creating an alias (with the correct linkage and name) to the 
original function instead?



cfe-commits mailing list

Reply via email to