does generate_tls_wrapper also need to be suppressed for aux module? David
On Thu, Mar 19, 2015 at 4:11 PM, Teresa Johnson <tejohn...@google.com> wrote: > New patch below. Passes regression tests plus internal application build. > > 2015-03-19 Teresa Johnson <tejohn...@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 <tejohn...@google.com> wrote: >> On Wed, Mar 18, 2015 at 9:25 PM, Xinliang David Li <davi...@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 <tejohn...@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 <tejohn...@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 | tejohn...@google.com | 408-460-2413 >>> >>> >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413