Fix Darwin 'constinit thread_local' variables.

[jyknight]

Summary

Unlike other platforms using ItaniumCXXABI, Darwin does not allow the
creation of a thread-wrapper function for a variable in the TU of
users. Because of this, it can set the linkage of the thread-local
symbol to internal, with the assumption that no TUs other than the one
defining the variable will need it.

However, constinit thread_local variables do not require the use of
the thread-wrapper call, so users reference the variable
directly. Thus, it must not be converted to internal.

[rjmccall]

clang/lib/CodeGen/CodeGenModule.cpp

4148

Is this a good idea? It seems to be making constinit part of the interface of a declaration, when it seems like it should strictly part of the implementation.

[hubert.reinterpretcast]

clang/lib/CodeGen/CodeGenModule.cpp

4148

@rsmith, fyi. My understanding is that constinit is as much part of the interface as __thread or _Thread_local.

[rjmccall]

LGTM

clang/lib/CodeGen/CodeGenModule.cpp

4148

Oh, heh. My impression had been an informal one, that it was purely a matter of enforcing static initialization, but I see that the committee did indeed sneak in the thread-local optimization. I guess all it means is that you can never take away constinit from an extern declaration once you've added it if you need a stable ABI.

Anyway, nevermind.

clang/lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
   if (const SectionAttr *SA = D->getAttr<SectionAttr>()) {
     const ASTContext::SectionInfo &SI = Context.SectionInfos[SA->getName()];
     if ((SI.SectionFlags & ASTContext::PSF_Write) == 0)
       GV->setConstant(true);
   }
 
   GV->setAlignment(getContext().getDeclAlign(D).getAsAlign());
 
-  // On Darwin, if the normal linkage of a C++ thread_local variable is
-  // LinkOnce or Weak, we keep the normal linkage to prevent multiple
-  // copies within a linkage unit; otherwise, the backing variable has
-  // internal linkage and all accesses should just be calls to the
-  // Itanium-specified entry point, which has the normal linkage of the
-  // variable. This is to preserve the ability to change the implementation
-  // behind the scenes.
-  if (!D->isStaticLocal() && D->getTLSKind() == VarDecl::TLS_Dynamic &&
+  // On Darwin, unlike other Itanium C++ ABI platforms, the thread-wrapper
+  // function is only defined alongside the variable, not also alongside
+  // callers. Normally, all accesses to a thread_local go through the
+  // thread-wrapper in order to ensure initialization has occurred, underlying
+  // variable will never be used other than the thread-wrapper, so it can be
+  // converted to internal linkage.
+  //
+  // However, if the variable has the 'constinit' attribute, it _can_ be
+  // referenced directly, without calling the thread-wrapper, so the linkage
+  // must not be changed.
+  //
+  // Additionally, if the variable isn't plain external linkage, e.g. if it's
+  // weak or linkonce, the de-duplication semantics are important to preserve,
+  // so we don't change the linkage.
+  if (D->getTLSKind() == VarDecl::TLS_Dynamic &&
+      Linkage == llvm::GlobalValue::ExternalLinkage &&
       Context.getTargetInfo().getTriple().isOSDarwin() &&
-      !llvm::GlobalVariable::isLinkOnceLinkage(Linkage) &&
-      !llvm::GlobalVariable::isWeakLinkage(Linkage))
+      !D->hasAttr<ConstInitAttr>())
     Linkage = llvm::GlobalValue::InternalLinkage;
 
   GV->setLinkage(Linkage);
   if (D->hasAttr<DLLImportAttr>())
     GV->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
   else if (D->hasAttr<DLLExportAttr>())
     GV->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
   else

clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp

-// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++2a %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++2a %s -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=LINUX %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin12  -std=c++2a %s -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=DARWIN %s
 
-// CHECK-DAG: @a = external thread_local global i32
-extern thread_local int a;
+// Check variable definitions/declarations. Note that on Darwin, typically the
+// variable's symbol is marked internal, and only the _ZTW function is
+// exported. Except: constinit variables do get exported, even on darwin.
 
-// CHECK-DAG: @b = external thread_local global i32
+// CHECK-DAG:  @a = external thread_local global i32
+// CHECK-DAG:  @b = external thread_local global i32
+// LINUX-DAG:  @c = thread_local global i32 0, align 4
+// DARWIN-DAG: @c = internal thread_local global i32 0, align 4
+// LINUX-DAG:  @d = thread_local global i32 0, align 4
+// DARWIN-DAG: @d = internal thread_local global i32 0, align 4
+// CHECK-DAG:  @e = external thread_local global %struct.Destructed, align 4
+// CHECK-DAG:  @e2 = thread_local global %struct.Destructed zeroinitializer, align 4
+// CHECK-DAG:  @f = thread_local global i32 4, align 4
+
+extern thread_local int a;
 extern thread_local constinit int b;
 
-// CHECK-LABEL: define i32 @_Z1fv()
-// CHECK: call i32* @_ZTW1a()
+// CHECK-LABEL: define i32 @_Z5get_av()
+// CHECK: call {{(cxx_fast_tlscc )?}}i32* @_ZTW1a()
 // CHECK: }
-int f() { return a; }
+int get_a() { return a; }
 
-// CHECK-LABEL: define linkonce_odr {{.*}} @_ZTW1a()
-// CHECK: br i1
-// CHECK: call void @_ZTH1a()
-// CHECK: }
+// LINUX-LABEL: define linkonce_odr {{.*}} @_ZTW1a()
+// LINUX: br i1
+// LINUX: call void @_ZTH1a()
+// LINUX: }
+// DARWIN-NOT: define {{.*}}@_ZTW1a()
 
-// CHECK-LABEL: define i32 @_Z1gv()
+// CHECK-LABEL: define i32 @_Z5get_bv()
 // CHECK-NOT: call
 // CHECK: load i32, i32* @b
 // CHECK-NOT: call
 // CHECK: }
-int g() { return b; }
+int get_b() { return b; }
 
 // CHECK-NOT: define {{.*}} @_ZTW1b()
 
 extern thread_local int c;
 
-// CHECK-LABEL: define i32 @_Z1hv()
-// CHECK: call i32* @_ZTW1c()
+// CHECK-LABEL: define i32 @_Z5get_cv()
+// LINUX: call {{(cxx_fast_tlscc )?}}i32* @_ZTW1c()
 // CHECK: load i32, i32* %
 // CHECK: }
-int h() { return c; }
+int get_c() { return c; }
 
 // Note: use of 'c' does not trigger initialization of 'd', because 'c' has a
 // constant initializer.
-// CHECK-LABEL: define weak_odr {{.*}} @_ZTW1c()
+// DARWIN-LABEL: define cxx_fast_tlscc {{.*}} @_ZTW1c()
+// LINUX-LABEL: define weak_odr {{.*}} @_ZTW1c()
 // CHECK-NOT: br i1
 // CHECK-NOT: call
 // CHECK: ret i32* @c
 // CHECK: }
 
 thread_local int c = 0;
 
 int d_init();
 
 // CHECK: define {{.*}}[[D_INIT:@__cxx_global_var_init[^(]*]](
 // CHECK: call {{.*}} @_Z6d_initv()
 thread_local int d = d_init();
 
 struct Destructed {
   int n = 0;
   ~Destructed();
 };
 
 extern thread_local constinit Destructed e;
-// CHECK-LABEL: define i32 @_Z1iv()
+// CHECK-LABEL: define i32 @_Z5get_ev()
 // CHECK: call {{.*}}* @_ZTW1e()
 // CHECK: }
-int i() { return e.n; }
+int get_e() { return e.n; }
 
 // CHECK: define {{.*}}[[E2_INIT:@__cxx_global_var_init[^(]*]](
-// CHECK: call {{.*}} @__cxa_thread_atexit({{.*}} @_ZN10DestructedD1Ev {{.*}} @e2
+// LINUX: call {{.*}} @__cxa_thread_atexit({{.*}} @_ZN10DestructedD1Ev {{.*}} @e2
+// DARWIN: call {{.*}} @_tlv_atexit({{.*}} @_ZN10DestructedD1Ev {{.*}} @e2
 thread_local constinit Destructed e2;
 
+thread_local constinit int f = 4;
+
 // CHECK-LABEL: define {{.*}}__tls_init
 // CHECK: call {{.*}} [[D_INIT]]
 // CHECK: call {{.*}} [[E2_INIT]]