Page MenuHomePhabricator

thread_local support for AIX

Authored by jamieschmeiser on Jun 16 2021, 2:08 PM.



Changes to support thread_local storage on AIX.

The AIX linker will produce errors on unresolved weak symbols. Change the
generated code to not check for the initialization function but just call
it and ensure that it always exists. Also, the AIX atexit routine has a
different name (and signature) so call it correctly. Update the lit tests
to test on AIX appropriately.

Diff Detail

Event Timeline

jamieschmeiser requested review of this revision.Jun 16 2021, 2:08 PM
jamieschmeiser created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 16 2021, 2:08 PM

Fix formatting problem.


Fix typo. Also, if the Linux and the AIX pattern is the same, adding a NONDARWIN prefix could help.


Can use {{.*}} in place of comdat to common up the Linux and the AIX pattern.


Minor nit: I think this should be moved to after line 5.


Same comment as for the earlier file re: adding a prefix to handle the common Linux and AIX lines.


Same comment as earlier about comdat.

Respond to review comments: Update to tests to create common LINUX/AIX portions.


The function registered needs to be something more like what createAtExitStub stub creates. Otherwise, the destructor will not be able to reference *this correctly.

As it is, the registered function is currently just the destructor itself:

$ clang++ -target powerpc64-ibm-aix -emit-llvm -S -o - -xc++ -<<<$'struct A { ~A(); }; thread_local A a;' | grep -C2 __pt_atexit_np
define internal void @__cxx_global_var_init() #0 {
  %0 = call i32 (i32, i32 (i32, ...)*, ...) @__pt_atexit_np(i32 0, i32 (i32, ...)* bitcast (void (%struct.A*)* @_ZN1AD1Ev to i32 (i32, ...)*)) #3
  ret void
declare void @_ZN1AD1Ev(%struct.A* nonnull align 1 dereferenceable(1)) unnamed_addr #1

declare i32 @__pt_atexit_np(i32, i32 (i32, ...)*, ...)

; Function Attrs: noinline
Return:  0x00:0   Mon Jul 12 15:24:15 2021 EDT

Minor nit: The coding guidelines have been updated for some time to discourage mixed bracing on if/else chains (see example

// Use braces for the `if` block to keep it uniform with the else block.



Just noting that the callback function registered by the IBM XL compiler does not make an effort to return 0 (but the documentation says the handler "must" return 0:


Clarify that this statement does not apply if the type is not a class type or (possibly multi-dimenional) array of class type and the variable is also constinit.


The linkage of the init function here should not be hard-coded as weak: If the strong definition of the variable has no dynamic initialization/finalization, then the init function the wrapper functions call should resolve to the empty version created here.


Unfortunately needsDestruction cannot be counted on at this time for some incomplete types, see


This comment block is a copy/paste from above and is meant to describe a case where only a declaration of the init func is being emitted into the IR.

Respond to review comments: create stub functions, return 0 from function and other fixes

Respond to review comments: update comments and make functions not weak


I think it is okay to leave the code as-is as it will then be fixed when that problem is fixed.

Remove accidental inclusion


Since the function has some specifics about the stub function type and return value behaviour:


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.


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.


This does not say that a thread_local variable of type int that is not constinit does have a guaranteed init routine.

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.


Since the code to generate the stub is new, I believe inspecting the body of the stub in the test would be appropriate.

Respond to review comments, formatting: fix comments, add assertion, fix
linkage, expand test to include body of generated function.

I think we're good after some last updates.


See suggested change for renaming the parameter name from IgnoreAttrs to InspectInitForWeakDef.


The block here is meant to be entered only if there is a definition of the variable (even if weak) in the current translation unit. This is mostly enforced with isEmittedWithConstantInitializer, but that will return true for constinit even if only a declaration is available.


Yes: I think (if the code works correctly), the needsDestruction query here only becomes significant when the type is complete -- and, in that case, we want it to work the way it currently does in relation to classes.

Respond to review comments: Change parameter name and tighten up conditions.

This revision is now accepted and ready to land.Fri, Jul 16, 4:28 PM
This revision was landed with ongoing or failed builds.Mon, Jul 19, 7:04 AM
This revision was automatically updated to reflect the committed changes.