This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode
- From: Xinliang David Li <davidxl at google dot com>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 19 Mar 2015 16:45:57 -0700
- Subject: Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode
- Authentication-results: sourceware.org; auth=none
- References: <CAAe5K+WnYun4Jkcq=tgjTz_iXaYeLBa8w-NLDji37zv6WSu_mg at mail dot gmail dot com> <CAAkRFZJrvb2X+s=BJpHwaCrBuzvUCNh5GAB0fGZS_VbvPOveyw at mail dot gmail dot com> <CAAe5K+WNCaD6BxTmnTp3P44jxrkyQN6Qj1ooPLU4qwMewgRu4w at mail dot gmail dot com> <CAAe5K+VqS_EtXX2pX9JE0E7kEh-+H6AzrCKN8Do9eYwTkskqJQ at mail dot gmail dot com>
does generate_tls_wrapper also need to be suppressed for aux module?
David
On Thu, Mar 19, 2015 at 4:11 PM, Teresa Johnson <tejohnson@google.com> wrote:
> New patch below. Passes regression tests plus internal application build.
>
> 2015-03-19 Teresa Johnson <tejohnson@google.com>
>
> gcc/
> Google ref b/19618364.
> * cp/decl2.c (get_local_tls_init_fn): Assert on aux module.
> (get_tls_init_fn): Promote non-public init functions if necessary
> in LIPO mode, record new global at module scope.
> (get_tls_wrapper_fn): Record new global at module scope.
> (handle_tls_init): Skip in aux module, setup alias in exported
> primary module.
>
> testsuite/
> Google ref b/19618364.
> * testsuite/g++.dg/tree-prof/lipo/tls.h: New test.
> * testsuite/g++.dg/tree-prof/lipo/tls2.h: Ditto.
> * testsuite/g++.dg/tree-prof/lipo/tls2_0.C: Ditto.
> * testsuite/g++.dg/tree-prof/lipo/tls2_1.C: Ditto.
> * testsuite/g++.dg/tree-prof/lipo/tls_0.C: Ditto.
> * testsuite/g++.dg/tree-prof/lipo/tls_1.C: Ditto.
>
> Index: cp/decl2.c
> ===================================================================
> --- cp/decl2.c (revision 221425)
> +++ cp/decl2.c (working copy)
> @@ -55,6 +55,7 @@ along with GCC; see the file COPYING3. If not see
> #include "langhooks.h"
> #include "c-family/c-ada-spec.h"
> #include "asan.h"
> +#include "gcov-io.h"
>
> extern cpp_reader *parse_in;
>
> @@ -3024,6 +3025,9 @@ var_needs_tls_wrapper (tree var)
> static tree
> get_local_tls_init_fn (void)
> {
> + /* In LIPO mode we should not generate local init functions for
> + the aux modules (see handle_tls_init). */
> + gcc_assert (!L_IPO_IS_AUXILIARY_MODULE);
> tree sname = get_identifier ("__tls_init");
> tree fn = IDENTIFIER_GLOBAL_VALUE (sname);
> if (!fn)
> @@ -3057,14 +3061,39 @@ get_tls_init_fn (tree var)
> if (!flag_extern_tls_init && DECL_EXTERNAL (var))
> return NULL_TREE;
>
> + /* In LIPO mode we should not generate local init functions for
> + aux modules. The wrapper will reference the variable's init function
> + that is defined in its own primary module. Otherwise there is
> + a name conflict between the primary and aux __tls_init functions,
> + and difficulty ensuring each TLS variable is initialized exactly once.
> + Therefore, if this is an aux module or an exported primary module, we
> + need to ensure that the init function is available externally by
> + promoting it if it is not already public. This is similar to the
> + handling in promote_static_var_func, but we do it as the init function
> + is created to avoid the need to detect and properly promote this
> + artificial decl later on. */
> + bool promote = L_IPO_IS_AUXILIARY_MODULE ||
> + (L_IPO_COMP_MODE && PRIMARY_MODULE_EXPORTED);
> +
> #ifdef ASM_OUTPUT_DEF
> /* If the variable is internal, or if we can't generate aliases,
> - call the local init function directly. */
> - if (!TREE_PUBLIC (var))
> + call the local init function directly. Don't do this if we
> + are in LIPO mode an may need to export the init function. Note
> + that get_local_tls_init_fn will assert if it is called on an aux
> + module. */
> + if (!TREE_PUBLIC (var) && !promote)
> #endif
> return get_local_tls_init_fn ();
>
> tree sname = mangle_tls_init_fn (var);
> + if (promote)
> + {
> + const char *name = IDENTIFIER_POINTER (sname);
> + char *assembler_name = (char*) alloca (strlen (name) + 30);
> + sprintf (assembler_name, "%s.cmo.%u", name, current_module_id);
> + sname = get_identifier (assembler_name);
> + }
> +
> tree fn = IDENTIFIER_GLOBAL_VALUE (sname);
> if (!fn)
> {
> @@ -3072,7 +3101,18 @@ get_tls_init_fn (tree var)
> build_function_type (void_type_node,
> void_list_node));
> SET_DECL_LANGUAGE (fn, lang_c);
> - TREE_PUBLIC (fn) = TREE_PUBLIC (var);
> + if (promote)
> + {
> + TREE_PUBLIC (fn) = 1;
> + DECL_VISIBILITY (fn) = VISIBILITY_HIDDEN;
> + DECL_VISIBILITY_SPECIFIED (fn) = 1;
> + }
> + else
> + {
> + TREE_PUBLIC (fn) = TREE_PUBLIC (var);
> + DECL_VISIBILITY (fn) = DECL_VISIBILITY (var);
> + DECL_VISIBILITY_SPECIFIED (fn) = DECL_VISIBILITY_SPECIFIED (var);
> + }
> DECL_ARTIFICIAL (fn) = true;
> DECL_COMDAT (fn) = DECL_COMDAT (var);
> DECL_EXTERNAL (fn) = DECL_EXTERNAL (var);
> @@ -3091,8 +3131,6 @@ get_tls_init_fn (tree var)
> else
> DECL_WEAK (fn) = DECL_WEAK (var);
> }
> - DECL_VISIBILITY (fn) = DECL_VISIBILITY (var);
> - DECL_VISIBILITY_SPECIFIED (fn) = DECL_VISIBILITY_SPECIFIED (var);
> DECL_DLLIMPORT_P (fn) = DECL_DLLIMPORT_P (var);
> DECL_IGNORED_P (fn) = 1;
> mark_used (fn);
> @@ -3100,6 +3138,11 @@ get_tls_init_fn (tree var)
> DECL_BEFRIENDING_CLASSES (fn) = var;
>
> SET_IDENTIFIER_GLOBAL_VALUE (sname, fn);
> + /* In LIPO mode make sure we record the new global value so that it
> + is cleared before parsing the next aux module. */
> + if (L_IPO_COMP_MODE && !is_parsing_done_p ())
> + add_decl_to_current_module_scope (fn,
> + NAMESPACE_LEVEL (global_namespace));
> }
> return fn;
> }
> @@ -3157,6 +3200,11 @@ get_tls_wrapper_fn (tree var)
> DECL_BEFRIENDING_CLASSES (fn) = var;
>
> SET_IDENTIFIER_GLOBAL_VALUE (sname, fn);
> + /* In LIPO mode make sure we record the new global value so that it
> + is cleared before parsing the next aux module. */
> + if (L_IPO_COMP_MODE && !is_parsing_done_p ())
> + add_decl_to_current_module_scope (fn,
> + NAMESPACE_LEVEL (global_namespace));
> }
> return fn;
> }
> @@ -4179,6 +4227,14 @@ clear_decl_external (struct cgraph_node *node, voi
> static void
> handle_tls_init (void)
> {
> + /* In LIPO mode we should not generate local init functions for
> + aux modules. The wrapper will reference the variable's init function
> + that is defined in its own primary module. Otherwise there is
> + a name conflict between the primary and aux __tls_init functions,
> + and difficulty ensuring each TLS variable is initialized exactly once. */
> + if (L_IPO_IS_AUXILIARY_MODULE)
> + return;
> +
> tree vars = prune_vars_needing_no_initialization (&tls_aggregates);
> if (vars == NULL_TREE)
> return;
> @@ -4213,8 +4269,12 @@ handle_tls_init (void)
> one_static_initialization_or_destruction (var, init, true);
>
> #ifdef ASM_OUTPUT_DEF
> - /* Output init aliases even with -fno-extern-tls-init. */
> - if (TREE_PUBLIC (var))
> + /* Output init aliases even with -fno-extern-tls-init. Also
> + export in LIPO mode if the primary module may be exported,
> + in which case the init function may be referenced externally
> + (see comments in get_tls_init_fn). */
> + if (TREE_PUBLIC (var) ||
> + (L_IPO_COMP_MODE && PRIMARY_MODULE_EXPORTED))
> {
> tree single_init_fn = get_tls_init_fn (var);
> if (single_init_fn == NULL_TREE)
> Index: testsuite/g++.dg/tree-prof/lipo/tls.h
> ===================================================================
> --- testsuite/g++.dg/tree-prof/lipo/tls.h (revision 0)
> +++ testsuite/g++.dg/tree-prof/lipo/tls.h (working copy)
> @@ -0,0 +1,16 @@
> +extern int NextId();
> +
> +class TLSClass {
> + public:
> + TLSClass() {
> + id = NextId();
> + bar = 1;
> + }
> + ~TLSClass() {}
> + int id;
> + int bar;
> +};
> +extern TLSClass* NextTLSClass();
> +extern void *SetTLSClass(TLSClass *a);
> +extern TLSClass *GetTLSClass();
> +extern thread_local TLSClass* current_tls_;
> Index: testsuite/g++.dg/tree-prof/lipo/tls2.h
> ===================================================================
> --- testsuite/g++.dg/tree-prof/lipo/tls2.h (revision 0)
> +++ testsuite/g++.dg/tree-prof/lipo/tls2.h (working copy)
> @@ -0,0 +1,15 @@
> +extern int NextId();
> +
> +class TLSClass {
> + public:
> + TLSClass() {
> + id = NextId();
> + bar = 1;
> + }
> + ~TLSClass() {}
> + int id;
> + int bar;
> +};
> +extern TLSClass* NextTLSClass();
> +extern void *SetTLSClass(TLSClass *a);
> +extern TLSClass *GetTLSClass();
> Index: testsuite/g++.dg/tree-prof/lipo/tls2_0.C
> ===================================================================
> --- testsuite/g++.dg/tree-prof/lipo/tls2_0.C (revision 0)
> +++ testsuite/g++.dg/tree-prof/lipo/tls2_0.C (working copy)
> @@ -0,0 +1,10 @@
> +// { dg-options "-std=c++11 -O2 --param=lipo-sampling-period=1" }
> +#include "tls2.h"
> +
> +static thread_local TLSClass* current_tls_ = NextTLSClass();
> +void *SetTLSClass(TLSClass *a) {
> + current_tls_ = a;
> +}
> +TLSClass *GetTLSClass() {
> + return current_tls_;
> +}
> Index: testsuite/g++.dg/tree-prof/lipo/tls2_1.C
> ===================================================================
> --- testsuite/g++.dg/tree-prof/lipo/tls2_1.C (revision 0)
> +++ testsuite/g++.dg/tree-prof/lipo/tls2_1.C (working copy)
> @@ -0,0 +1,26 @@
> +// { dg-options "-std=c++11 -O2 --param=lipo-sampling-period=1" }
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <new>
> +#include "tls2.h"
> +TLSClass* NextTLSClass() {
> + return new TLSClass();
> +}
> +int NextId() {
> + static int id = 0;
> + return id++;
> +}
> +static thread_local TLSClass* current_tls2_ = NextTLSClass();
> +void *SetTLSClass2(TLSClass *a) {
> + current_tls2_ = a;
> +}
> +int main() {
> + printf ("Id %d\n", GetTLSClass()->id);
> + TLSClass *A = NextTLSClass();
> + SetTLSClass(A);
> + printf ("Id %d\n", GetTLSClass()->id);
> + printf ("Id %d\n", current_tls2_->id);
> + A = NextTLSClass();
> + SetTLSClass2(A);
> + printf ("Id %d\n", current_tls2_->id);
> +}
> Index: testsuite/g++.dg/tree-prof/lipo/tls_0.C
> ===================================================================
> --- testsuite/g++.dg/tree-prof/lipo/tls_0.C (revision 0)
> +++ testsuite/g++.dg/tree-prof/lipo/tls_0.C (working copy)
> @@ -0,0 +1,10 @@
> +// { dg-options "-std=c++11 -O2 --param=lipo-sampling-period=1" }
> +#include "tls.h"
> +
> +thread_local TLSClass* current_tls_ = NextTLSClass();
> +void *SetTLSClass(TLSClass *a) {
> + current_tls_ = a;
> +}
> +TLSClass *GetTLSClass() {
> + return current_tls_;
> +}
> Index: testsuite/g++.dg/tree-prof/lipo/tls_1.C
> ===================================================================
> --- testsuite/g++.dg/tree-prof/lipo/tls_1.C (revision 0)
> +++ testsuite/g++.dg/tree-prof/lipo/tls_1.C (working copy)
> @@ -0,0 +1,32 @@
> +// { dg-options "-std=c++11 -O2 --param=lipo-sampling-period=1" }
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <new>
> +#include "tls.h"
> +TLSClass* NextTLSClass() {
> + return new TLSClass();
> +}
> +int NextId() {
> + static int id = 0;
> + return id++;
> +}
> +void *SetTLSClassHere(TLSClass *a) {
> + current_tls_ = a;
> +}
> +thread_local TLSClass* current_tls2_ = NextTLSClass();
> +void *SetTLSClass2(TLSClass *a) {
> + current_tls2_ = a;
> +}
> +int main() {
> + printf ("Id %d\n", GetTLSClass()->id);
> + TLSClass *A = NextTLSClass();
> + SetTLSClass(A);
> + printf ("Id %d\n", GetTLSClass()->id);
> + A = NextTLSClass();
> + SetTLSClassHere(A);
> + printf ("Id %d\n", GetTLSClass()->id);
> + printf ("Id %d\n", current_tls2_->id);
> + A = NextTLSClass();
> + SetTLSClass2(A);
> + printf ("Id %d\n", current_tls2_->id);
> +}
>
> On Thu, Mar 19, 2015 at 6:09 AM, Teresa Johnson <tejohnson@google.com> wrote:
>> On Wed, Mar 18, 2015 at 9:25 PM, Xinliang David Li <davidxl@google.com> wrote:
>>> get_local_tls_init_fn can be called from other contexts other than
>>> 'handle_tls_init' -- is the added new assertion safe ?
>>
>> In fact there is still an issue here, for file static TLS vars. Will
>> work on a fix and send the original test case (forgot to include in
>> first patch) and the new one with the fixed patch.
>>
>> Teresa
>>
>>>
>>> David
>>>
>>> On Wed, Mar 18, 2015 at 9:18 PM, Teresa Johnson <tejohnson@google.com>
>>> wrote:
>>>>
>>>> Ensure TLS variable wrapper and init functions are recorded at
>>>> the module scope upon creation so that they are cleared when
>>>> popping the module scope in between modules in LIPO mode.
>>>>
>>>> Also, do not generate a local TLS init function (__tls_init)
>>>> for aux functions, only in the primary modules.
>>>>
>>>> Passes regression tests. Ok for Google branches?
>>>> Teresa
>>>>
>>>> 2015-03-18 Teresa Johnson <tejohnson@google.com>
>>>>
>>>> Google ref b/19618364.
>>>> * cp/decl2.c (get_local_tls_init_fn): Assert on aux module.
>>>> (get_tls_init_fn): Record new global decl.
>>>> (get_tls_wrapper_fn): Ditto.
>>>> (handle_tls_init): Skip aux modules.
>>>>
>>>> Index: cp/decl2.c
>>>> ===================================================================
>>>> --- cp/decl2.c (revision 221425)
>>>> +++ cp/decl2.c (working copy)
>>>> @@ -3036,6 +3036,9 @@ get_local_tls_init_fn (void)
>>>> DECL_ARTIFICIAL (fn) = true;
>>>> mark_used (fn);
>>>> SET_IDENTIFIER_GLOBAL_VALUE (sname, fn);
>>>> + /* In LIPO mode we should not generate local init functions for
>>>> + the aux modules (see handle_tls_init). */
>>>> + gcc_assert (!L_IPO_IS_AUXILIARY_MODULE);
>>>> }
>>>> return fn;
>>>> }
>>>> @@ -3100,6 +3103,11 @@ get_tls_init_fn (tree var)
>>>> DECL_BEFRIENDING_CLASSES (fn) = var;
>>>>
>>>> SET_IDENTIFIER_GLOBAL_VALUE (sname, fn);
>>>> + /* In LIPO mode make sure we record the new global value so that it
>>>> + is cleared before parsing the next aux module. */
>>>> + if (L_IPO_COMP_MODE && !is_parsing_done_p ())
>>>> + add_decl_to_current_module_scope (fn,
>>>> + NAMESPACE_LEVEL
>>>> (global_namespace));
>>>> }
>>>> return fn;
>>>> }
>>>> @@ -3157,6 +3165,11 @@ get_tls_wrapper_fn (tree var)
>>>> DECL_BEFRIENDING_CLASSES (fn) = var;
>>>>
>>>> SET_IDENTIFIER_GLOBAL_VALUE (sname, fn);
>>>> + /* In LIPO mode make sure we record the new global value so that it
>>>> + is cleared before parsing the next aux module. */
>>>> + if (L_IPO_COMP_MODE && !is_parsing_done_p ())
>>>> + add_decl_to_current_module_scope (fn,
>>>> + NAMESPACE_LEVEL
>>>> (global_namespace));
>>>> }
>>>> return fn;
>>>> }
>>>> @@ -4179,6 +4192,14 @@ clear_decl_external (struct cgraph_node *node, voi
>>>> static void
>>>> handle_tls_init (void)
>>>> {
>>>> + /* In LIPO mode we should not generate local init functions for
>>>> + aux modules. The wrapper will reference the variable's init function
>>>> + that is defined in its own primary module. Otherwise there is
>>>> + a name conflict between the primary and aux __tls_init functions,
>>>> + and difficulty ensuring each TLS variable is initialized exactly
>>>> once. */
>>>> + if (L_IPO_IS_AUXILIARY_MODULE)
>>>> + return;
>>>> +
>>>> tree vars = prune_vars_needing_no_initialization (&tls_aggregates);
>>>> if (vars == NULL_TREE)
>>>> return;
>>>>
>>>>
>>>> --
>>>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>>>
>>>
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413