Fix Darwin 'constinit thread_local' variables.

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, or users will
get a link failure.

This was a regression introduced by the optimization in
0022382.

Differential Revision: https://reviews.llvm.org/D80417

Author: jyknight

clang/lib/CodeGen/CodeGenModule.cpp

@@ -4136,17 +4136,24 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
 
   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);

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

@@ -1,41 +1,55 @@
-// 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 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:  @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
 
-// CHECK-DAG: @a = external thread_local global i32
 extern thread_local int a;
-
-// CHECK-DAG: @b = external thread_local global i32
 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
@@ -55,15 +69,18 @@ struct 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]]