[PATCH] D104420: thread_local support for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:265
 
+/// Create a stub function, suitable for being passed to atexit,
+/// which passes the given address to the given destructor function.
----------------
Since the function has some specifics about the stub function type and return value behaviour:
s/atexit/__pt_atexit_np/;



================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2959
+        isEmittedWithConstantInitializer(VD, true) &&
+        !VD->needsDestruction(getContext())) {
+      llvm::Function *Func = llvm::Function::Create(
----------------
An assertion that `Init` is null should be appropriate here: If it is non-null, then the pre-existing logic above would either be defining the function to be an alias or is declaring the function with the expectation that the definition of the variable is elsewhere.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2961
+      llvm::Function *Func = llvm::Function::Create(
+          InitFnTy, llvm::GlobalVariable::ExternalLinkage, InitFnName.str(),
+          &CGM.getModule());
----------------
The linkage should be weak for a variable defined to be weak. For example, the code higher up uses `Var->getLinkage()` to produce the alias.


================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2988-2989
+    } else if (CGM.getTriple().isOSAIX()) {
+      // On AIX, thread_local vars, other than non-class types or (possibly
+      // multi-dimensional) arrays or such types, will have init routines
+      // regardless of whether they are const-initialized or not.  Since the
----------------
This does not say that a `thread_local` variable of type `int` that is not `constinit` //does// have a guaranteed init routine.

Suggestion:
On AIX, except if constinit and also neither of class type or of (possibly multi-dimensional) array of class type, thread_local vars will have init routines regardless of whether they are const-initialized.


================
Comment at: clang/test/CodeGenCXX/cxx11-thread-local.cpp:241
   // LINUX: call i32 @__cxa_thread_atexit({{.*}}@_ZN1SD1Ev {{.*}} @_ZZ8tls_dtorvE1s{{.*}} @__dso_handle
+  // AIX: call i32 (i32, i32 (i32, ...)*, ...) @__pt_atexit_np(i32 0, {{.*}}@__dtor__ZZ8tls_dtorvE1s){{.*}}
   // DARWIN: call i32 @_tlv_atexit({{.*}}@_ZN1SD1Ev {{.*}} @_ZZ8tls_dtorvE1s{{.*}} @__dso_handle
----------------
Since the code to generate the stub is new, I believe inspecting the body of the stub in the test would be appropriate.


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

https://reviews.llvm.org/D104420