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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits