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

Jason Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 30 12:42:31 PDT 2020


jasonliu added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:24
 #include "llvm/Support/Path.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
----------------
We are removing the usage of "getUniqueModuleId" in this file, So I assume this include could get removed as well.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+  /// Add an sterm finalizer to its own llvm.global_dtors entry.
+  void AddCXXStermFinalizerToGlobalDtor(llvm::Function *StermFinalizer,
+                                        int Priority) {
----------------
This wrapper seems redundant. Calling ` AddGlobalDtor(StermFinalizer, Priority);` directly from the callee side already convey what we are trying to achieve here. 


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4602
+    llvm::report_fatal_error(
+        "prioritized __sinit and __sterm functions are not yet supported");
+  else if (isTemplateInstantiation(D.getTemplateSpecializationKind()) ||
----------------
Could we trigger this error? If so, could we have a test for it? 


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2119
-    unsigned Priority, const MCSymbol *KeySym) const {
-  report_fatal_error("XCOFF dtor section not yet implemented.");
-}
----------------
I think it's still useful to keep these functions around to prevent accidentally calling to these functions on AIX. We could rephrase error message to "static constructor/destructor section is not needed on AIX."


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1874
+      }
+      emitSpecialLLVMGlobal(&G);
+      continue;
----------------
Have some suggestion on structure backend code change:

1. Remove `isSpecialLLVMGlobalArrayForStaticInit` and `isSpecialLLVMGlobalArrayToSkip`, and call `emitSpecialLLVMGlobal` directly instead. This would handle all the `llvm.` variable for us. We would need do early return for names start with `llvm.` in emitGlobalVariable as well, since they are all handled here.

2. We could override emitXXStructorList because how XCOFF handle the list is vastly different than the other target. We could make the common part(i.e. processing and sorting) a utility function, say "preprocessStructorList".

3. It's not necessary to use the same name `emitXXStructor` if the interface is different. It might not provide much help when we kinda know no other target is going to use this interface. So we could turn it into a lambda inside of the overrided emitXXStructorList, or simply no need for a new function because the logic is fairly straightforward.



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

https://reviews.llvm.org/D84534



More information about the cfe-commits mailing list