[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Jason Liu via Phabricator via cfe-commits]

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?


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

https://reviews.llvm.org/D84534