From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by sourceware.org (Postfix) with ESMTPS id C99623858D1E for ; Fri, 30 Dec 2022 09:07:07 +0000 (GMT) Received: by mail-wm1-x335.google.com with SMTP id k26-20020a05600c1c9a00b003d972646a7dso11729599wms.5 for ; Fri, 30 Dec 2022 01:07:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=RLHIn4gw11rfGwvGGRWXQqWIp1uD9AWX89pUDQbpzrQ=; b=YKXqPzKPIH7eMTwfbAcDXxgNx4jHY74FVXrwHMDJ0lSCUMmnMIE2V1ixasLQg2QWgU qCxv9xc/oHjsLu5FKr6SrzSNONI+ALtFX0ccoJvVGsPo+l0Zzp0GmOC6sYZm/3fmzlXo 51EvspkzW03ELdDAe9mPQ5DTeIjWkc+f36WV2l2z7VOBY9co/VmtHDk7E987V+BpjpAc iAYfYj6uI9oKcqPPOe1P1r5lldQ/8M+PGmLTUsIT70Myvnh9vNUG6AjN5AW3S23DkuqM AdyOZnCQn56QNkNxPpr5qmYhBDdbGSsC9pLZEv2q6UUmTPYbMtkBNR1OcFyd3QbICPTI 4g+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=RLHIn4gw11rfGwvGGRWXQqWIp1uD9AWX89pUDQbpzrQ=; b=FD/WyUKgLFVWzFkUllCkdUyCVDT6hkdpKCm3zt+8O2ucvNz7l/PrlyDEmkJyoqk0DR j+E6yLv/7OpUjW01Q55WHgUBZRZzSd4Po9FV8vUtQn4iy7G4fSd2Zk7w7X/udQSPfvY0 Y1FUf4Bsq6XY9H/OZ7SqDMlea7ViRh4OBPyIP0l/essKTgh5/BQ/27EzIw9EnNKkUG3w 2TdZOm24EbiLIiVgfeFtQHNiIdQ3CTEO9OcKRk+6suMd1JfNsbS5Mwceot06o0gCdYNL Yyq8XvT1KEZRv/MmzhuXE/maO+nDXHaw907R624PsInROOu3IP2lN20/WvbjVlllZQWX mEYA== X-Gm-Message-State: AFqh2kobJ4PSAMNk/HSV9+akAuvcDBVj5ADjMeOosV7xR4tcKDLmJ8Or 3uZFzpP3GzoJ3dMla6QsVT7POKhWpZk= X-Google-Smtp-Source: AMrXdXsF3KwrWGg9DCAqc/TS7iXFnSb4Ds25huG0oNMRnIg41upoFg+IKByBkNCUqA4a7w/UIabMUQ== X-Received: by 2002:a05:600c:16c5:b0:3d1:f687:1fd0 with SMTP id l5-20020a05600c16c500b003d1f6871fd0mr22161242wmn.12.1672391226141; Fri, 30 Dec 2022 01:07:06 -0800 (PST) Received: from smtpclient.apple (host81-138-1-83.in-addr.btopenworld.com. [81.138.1.83]) by smtp.gmail.com with ESMTPSA id o37-20020a05600c512500b003d98f92692fsm12868624wms.17.2022.12.30.01.07.05 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 30 Dec 2022 01:07:05 -0800 (PST) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\)) Subject: [ping^1] [PATCH] c++, TLS: Support cross-tu static initialization for targets without alias support [PR106435]. From: Iain Sandoe In-Reply-To: <20221207153939.49157-1-iain@sandoe.co.uk> Date: Fri, 30 Dec 2022 09:07:04 +0000 Cc: Jason Merrill Content-Transfer-Encoding: quoted-printable Message-Id: References: <20221207153939.49157-1-iain@sandoe.co.uk> To: GCC Patches X-Mailer: Apple Mail (2.3696.120.41.1.1) X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: it seems other targets are interested in this too - I was asked on IRC = if it was going to be fixed this cycle. > On 7 Dec 2022, at 15:39, Iain Sandoe via Gcc-patches = wrote: >=20 > This has been tested on x86_64 and arm64 Darwin and on x86_64 linux = gnu. > The basic patch is live in the homebrew macOS support and so has had = quite > wide coverage on non-trivial codebases. >=20 > OK for master? > Iain >=20 > Since this actually fixes wrong code, I wonder if we should also = consider > back-porting. >=20 > --- >8 --- >=20 > The description below relates to the code path when = TARGET_SUPPORTS_ALIASES is > false; current operation is maintained for targets with alias support = and any > new support code should be DCEd in that case. >=20 > -- >=20 > Currently, cross-tu static initialisation is not supported for targets = without > alias support. >=20 > The patch adds support by building a shim function in place of the = alias for > these targets; the shim simply calls the generic initialiser. = Although this is > slightly less efficient than the alias, in practice (for targets that = allow > sibcalls) the penalty is a single jump when code is optimised. >=20 > =46rom the perspective of a TU referencing an extern TLS variable, = there is no > way to determine if it requires a guarded dynamic init. So, in the = referencing > TU, we build a weak reference to the potential init and check at = runtime if the > init is present before calling it. This strategy is fine for targets = that have > ELF semantics, but fails at link time for Mach-O (which does not = permit the > reference to be undefined in the static link). >=20 > The actual initialiser call is contained in a wrapper function, and to = resolve > the Mach-O linker issue, in the TU that is referencing the var, we now = generate > both the wrapper _and_ a weak definition of a dummy init function. In = the case > that there _is_ a dynamic init (in a different TU), that version will = be non-weak > and will be override the weak dummy one. In the case that we have a = trivial > static init (so no init in any other TU) the weak-defined dummy init = will be > called (a single return insn for optimised code). We mitigate the = call to > the dummy init by reworking the wrapper code-gen path to remove the = test for > the weak reference function (as it will always be true) since the = static linker > will now determine the function to be called. >=20 > Signed-off-by: Iain Sandoe >=20 > PR c++/106435 >=20 > gcc/c-family/ChangeLog: >=20 > * c-opts.cc (c_common_post_options): Allow fextern-tls-init for = targets > without alias support. >=20 > gcc/cp/ChangeLog: >=20 > * decl2.cc (get_tls_init_fn): Allow targets without alias = support. > (handle_tls_init): Emit aliases for single init functions where = the > target supporst this, otherwise emit a stub function that calls = the > main tls init function. (generate_tls_dummy_init): New. >=20 > gcc/testsuite/ChangeLog: >=20 > * g++.dg/cpp0x/pr106435-b.cc: New file. > * g++.dg/cpp0x/pr106435.C: New test. > * g++.dg/cpp0x/pr106435.h: New file. > --- > gcc/c-family/c-opts.cc | 2 +- > gcc/cp/decl2.cc | 80 ++++++++++++++++++++---- > gcc/testsuite/g++.dg/cpp0x/pr106435-b.cc | 22 +++++++ > gcc/testsuite/g++.dg/cpp0x/pr106435.C | 24 +++++++ > gcc/testsuite/g++.dg/cpp0x/pr106435.h | 27 ++++++++ > 5 files changed, 142 insertions(+), 13 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr106435-b.cc > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr106435.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr106435.h >=20 > diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc > index 70745aa4e7c..064645f980d 100644 > --- a/gcc/c-family/c-opts.cc > +++ b/gcc/c-family/c-opts.cc > @@ -1070,7 +1070,7 @@ c_common_post_options (const char **pfilename) >=20 > if (flag_extern_tls_init) > { > - if (!TARGET_SUPPORTS_ALIASES || !SUPPORTS_WEAK) > + if (!SUPPORTS_WEAK) > { > /* Lazy TLS initialization for a variable in another TU = requires > alias and weak reference support. */ > diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc > index f95529a5c9a..c6550c0c2fc 100644 > --- a/gcc/cp/decl2.cc > +++ b/gcc/cp/decl2.cc > @@ -3672,9 +3672,8 @@ get_tls_init_fn (tree var) > if (!flag_extern_tls_init && DECL_EXTERNAL (var)) > return NULL_TREE; >=20 > - /* If the variable is internal, or if we can't generate aliases, > - call the local init function directly. */ > - if (!TREE_PUBLIC (var) || !TARGET_SUPPORTS_ALIASES) > + /* If the variable is internal call the local init function = directly. */ > + if (!TREE_PUBLIC (var)) > return get_local_tls_init_fn (DECL_SOURCE_LOCATION (var)); >=20 > tree sname =3D mangle_tls_init_fn (var); > @@ -3811,8 +3810,12 @@ generate_tls_wrapper (tree fn) > if (tree init_fn =3D get_tls_init_fn (var)) > { > tree if_stmt =3D NULL_TREE; > - /* If init_fn is a weakref, make sure it exists before calling. = */ > - if (lookup_attribute ("weak", DECL_ATTRIBUTES (init_fn))) > + > + /* If init_fn is a weakref, make sure it exists before calling. > + If the target does not support aliases, then we will have = generated > + a dummy weak function, so there is no need to test its = existence. */ > + if (TARGET_SUPPORTS_ALIASES && > + lookup_attribute ("weak", DECL_ATTRIBUTES (init_fn))) > { > if_stmt =3D begin_if_stmt (); > tree addr =3D cp_build_addr_expr (init_fn, = tf_warning_or_error); > @@ -3837,6 +3840,25 @@ generate_tls_wrapper (tree fn) > expand_or_defer_fn (finish_function (/*inline_p=3D*/false)); > } >=20 > +/* A dummy init function to act as a weak placeholder for a (possibly = non- > + existent) dynamic init. */ > +static void > +generate_tls_dummy_init (tree fn) > +{ > + tree var =3D DECL_BEFRIENDING_CLASSES (fn); > + tree init_fn =3D get_tls_init_fn (var); > + /* If have no init fn, or it is non-weak, then we do not need to = make a > + dummy. */ > + if (!init_fn || !lookup_attribute ("weak", DECL_ATTRIBUTES = (init_fn))) > + return; > + start_preparsed_function (init_fn, NULL_TREE, SF_DEFAULT | = SF_PRE_PARSED); > + tree body =3D begin_function_body (); > + declare_weak (init_fn); > + finish_return_stmt (NULL_TREE); > + finish_function_body (body); > + expand_or_defer_fn (finish_function (/*inline_p=3D*/false)); > +} > + > /* Start a global constructor or destructor function. */ >=20 > static tree > @@ -4626,22 +4648,24 @@ handle_tls_init (void) > finish_expr_stmt (cp_build_modify_expr (loc, guard, NOP_EXPR, > boolean_true_node, > tf_warning_or_error)); > + auto_vec direct_calls; > for (; vars; vars =3D TREE_CHAIN (vars)) > { > tree var =3D TREE_VALUE (vars); > tree init =3D TREE_PURPOSE (vars); > one_static_initialization_or_destruction (/*initp=3D*/true, var, = init); >=20 > - /* Output init aliases even with -fno-extern-tls-init. */ > - if (TARGET_SUPPORTS_ALIASES && TREE_PUBLIC (var)) > + /* Output inits even with -fno-extern-tls-init. > + We save the list here and output either an alias or a stub = function > + below. */ > + if (TREE_PUBLIC (var)) > { > tree single_init_fn =3D get_tls_init_fn (var); > if (single_init_fn =3D=3D NULL_TREE) > continue; > - cgraph_node *alias > - =3D cgraph_node::get_create (fn)->create_same_body_alias > - (single_init_fn, fn); > - gcc_assert (alias !=3D NULL); > + if (single_init_fn =3D=3D fn) > + continue; > + direct_calls.safe_push (single_init_fn); > } > } >=20 > @@ -4649,6 +4673,30 @@ handle_tls_init (void) > finish_if_stmt (if_stmt); > finish_function_body (body); > expand_or_defer_fn (finish_function (/*inline_p=3D*/false)); > + > + /* For each TLS var that we have an init function, we either emit = an alias > + between that and the tls_init, or a stub function that just = calls the > + tls_init. */ > + while (!direct_calls.is_empty()) > + { > + tree single_init_fn =3D direct_calls.pop (); > + if (TARGET_SUPPORTS_ALIASES) > + { > + cgraph_node *alias > + =3D cgraph_node::get_create (fn)->create_same_body_alias > + (single_init_fn, fn); > + gcc_assert (alias !=3D NULL); > + } > + else > + { > + start_preparsed_function (single_init_fn, NULL_TREE, = SF_PRE_PARSED); > + tree body =3D begin_function_body (); > + tree r =3D build_call_expr (fn, 0); > + finish_expr_stmt (r); > + finish_function_body (body); > + expand_or_defer_fn (finish_function (/*inline_p=3D*/false)); > + } > + } > } >=20 > /* We're at the end of compilation, so generate any mangling aliases = that > @@ -5058,7 +5106,15 @@ c_parse_final_cleanups (void) > } >=20 > if (!DECL_INITIAL (decl) && decl_tls_wrapper_p (decl)) > - generate_tls_wrapper (decl); > + { > + generate_tls_wrapper (decl); > + if (!TARGET_SUPPORTS_ALIASES) > + /* The wrapper might have a weak reference to an init, we = provide > + a dummy function to satisfy that here. The = linker/dynamic > + loader will override this with the actual init, if one = is > + required. */ > + generate_tls_dummy_init (decl); > + } >=20 > if (!DECL_SAVED_TREE (decl)) > continue; > diff --git a/gcc/testsuite/g++.dg/cpp0x/pr106435-b.cc = b/gcc/testsuite/g++.dg/cpp0x/pr106435-b.cc > new file mode 100644 > index 00000000000..bd586286647 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/pr106435-b.cc > @@ -0,0 +1,22 @@ > +// PR c++/106435 > +#include "pr106435.h" > + > +//#include > + > +Foo::Foo() { > + ++num_calls; > +// std::cout << "Foo::Foo(this=3D" << this << ")\n"; > +} > + > +int Foo::func() { > +// std::cout << "Foo::func(this=3D" << this << ")\n"; > + return num_calls; > +} > + > +thread_local Foo Bar::foo; > +thread_local Foo Bar::baz; > + > +thread_local Foo F =3D Foo{5}; > + > +thread_local void* tl =3D (void*)&F; > +thread_local void* tl1 =3D nullptr; > diff --git a/gcc/testsuite/g++.dg/cpp0x/pr106435.C = b/gcc/testsuite/g++.dg/cpp0x/pr106435.C > new file mode 100644 > index 00000000000..9071e032697 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/pr106435.C > @@ -0,0 +1,24 @@ > +// PR c++/106435 > +// { dg-do run { target c++11 } } > +// { dg-additional-sources "pr106435-b.cc" } > + > +#include "pr106435.h" > + > +int num_calls =3D 0; > + > +thread_local Foo Bar::bat; > + > +int main() { > + int v =3D Bar::foo.func(); > + if (v !=3D 2) > + __builtin_abort (); > + v =3D Bar::bat.func(); > + if (v !=3D 3) > + __builtin_abort (); > + if (F.getX() !=3D 5) > + __builtin_abort(); > + if (gt_tl () !=3D (void*)&F) > + __builtin_abort(); > + if (gt_tl1 ()) > + __builtin_abort(); > +} > diff --git a/gcc/testsuite/g++.dg/cpp0x/pr106435.h = b/gcc/testsuite/g++.dg/cpp0x/pr106435.h > new file mode 100644 > index 00000000000..06d3e60484f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/pr106435.h > @@ -0,0 +1,27 @@ > +// PR c++/106435 > +#pragma once > + > +extern int num_calls; > +struct Foo { > + Foo(); > + Foo(int _x) : x(_x) {} > + int func(); > + int getX () { return x; } > + int x; > +}; > + > +struct Bar { > + thread_local static Foo foo; > + thread_local static Foo baz; > + thread_local static Foo bat; > +}; > + > +extern thread_local void* tl; > + > +inline void* gt_tl () {return tl;} > + > +extern thread_local Foo F; > + > +extern thread_local void* tl1; > + > +inline void* gt_tl1 () {return tl1;} > --=20 > 2.37.1 (Apple Git-137.1) >=20