[clang] aca3d06 - Fix Darwin 'constinit thread_local' variables.
James Y Knight via cfe-commits
cfe-commits at lists.llvm.org
Wed May 27 08:59:57 PDT 2020
Author: James Y Knight
Date: 2020-05-27T11:59:30-04:00
New Revision: aca3d067efe194539efd1e0fcf03820a2c377753
URL: https://github.com/llvm/llvm-project/commit/aca3d067efe194539efd1e0fcf03820a2c377753
DIFF: https://github.com/llvm/llvm-project/commit/aca3d067efe194539efd1e0fcf03820a2c377753.diff
LOG: 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
00223827a952f66e7426c9881a2a4229e59bb019.
Differential Revision: https://reviews.llvm.org/D80417
Added:
Modified:
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index f43bc6434daf..89a95db08680 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/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);
diff --git a/clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp b/clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
index f47707555098..99c5d721dc47 100644
--- a/clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
+++ b/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]]
More information about the cfe-commits
mailing list