* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode [not found] ` <CAAkRFZJrvb2X+s=BJpHwaCrBuzvUCNh5GAB0fGZS_VbvPOveyw@mail.gmail.com> @ 2015-03-19 13:09 ` Teresa Johnson 2015-03-19 23:11 ` Teresa Johnson 0 siblings, 1 reply; 10+ messages in thread From: Teresa Johnson @ 2015-03-19 13:09 UTC (permalink / raw) To: Xinliang David Li; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-19 13:09 ` [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode Teresa Johnson @ 2015-03-19 23:11 ` Teresa Johnson 2015-03-19 23:46 ` Xinliang David Li 0 siblings, 1 reply; 10+ messages in thread From: Teresa Johnson @ 2015-03-19 23:11 UTC (permalink / raw) To: Xinliang David Li; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-19 23:11 ` Teresa Johnson @ 2015-03-19 23:46 ` Xinliang David Li 2015-03-20 0:51 ` Teresa Johnson 0 siblings, 1 reply; 10+ messages in thread From: Xinliang David Li @ 2015-03-19 23:46 UTC (permalink / raw) To: Teresa Johnson; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-19 23:46 ` Xinliang David Li @ 2015-03-20 0:51 ` Teresa Johnson 2015-03-20 3:01 ` Xinliang David Li 0 siblings, 1 reply; 10+ messages in thread From: Teresa Johnson @ 2015-03-20 0:51 UTC (permalink / raw) To: Xinliang David Li; +Cc: gcc-patches On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: > does generate_tls_wrapper also need to be suppressed for aux module? No, we generate the wrapper in the aux module and use it to access the TLS variable and invoke it's init function (which is defined in the variable's own module). Presumably we could generate that only in the original module and promote it if necessary, but generating it in the aux module does allow it to be inlined. This is essentially how the TLS accesses are handled for extern TLS variables defined elsewhere (wrapper generated in referring module). Teresa > > 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 -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-20 0:51 ` Teresa Johnson @ 2015-03-20 3:01 ` Xinliang David Li 2015-03-20 4:57 ` Teresa Johnson 0 siblings, 1 reply; 10+ messages in thread From: Xinliang David Li @ 2015-03-20 3:01 UTC (permalink / raw) To: Teresa Johnson; +Cc: gcc-patches ok -- then there is an over assertion in get_local_tls_init_fn. The method can be called for aux module -- the decl created will also be promoted. Also can we rely on late promotion (special case of artificial function __tls_init? This can avoid duplicated logic here. David On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: >> does generate_tls_wrapper also need to be suppressed for aux module? > > No, we generate the wrapper in the aux module and use it to access the > TLS variable and invoke it's init function (which is defined in the > variable's own module). Presumably we could generate that only in the > original module and promote it if necessary, but generating it in the > aux module does allow it to be inlined. This is essentially how the > TLS accesses are handled for extern TLS variables defined elsewhere > (wrapper generated in referring module). > > Teresa > >> >> 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 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-20 3:01 ` Xinliang David Li @ 2015-03-20 4:57 ` Teresa Johnson 2015-03-20 5:38 ` Xinliang David Li 0 siblings, 1 reply; 10+ messages in thread From: Teresa Johnson @ 2015-03-20 4:57 UTC (permalink / raw) To: Xinliang David Li; +Cc: gcc-patches On Thu, Mar 19, 2015 at 8:00 PM, Xinliang David Li <davidxl@google.com> wrote: > ok -- then there is an over assertion in get_local_tls_init_fn. The > method can be called for aux module -- the decl created will also be > promoted. No, it won't ever be called for an aux module. Both callsites are guarded (handle_tls_init has an early return at the top, and the callsite is guarded in get_tls_iniit_fn). > > Also can we rely on late promotion (special case of artificial > function __tls_init? This can avoid duplicated logic here. We don't need to promote __tls_init, but rather the init functions for each TLS variable (which are each aliased to __tls_init). I thought about both approaches, but promoting it as it was created seemed more straightforward. I can give that approach a try though if you prefer it (special casing DECL_ARTIFICIAL functions that start with _ZTH). Incidentally they are not marked static, so that would also need to be done to get them promoted. Teresa > > David > > On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson <tejohnson@google.com> wrote: >> On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: >>> does generate_tls_wrapper also need to be suppressed for aux module? >> >> No, we generate the wrapper in the aux module and use it to access the >> TLS variable and invoke it's init function (which is defined in the >> variable's own module). Presumably we could generate that only in the >> original module and promote it if necessary, but generating it in the >> aux module does allow it to be inlined. This is essentially how the >> TLS accesses are handled for extern TLS variables defined elsewhere >> (wrapper generated in referring module). >> >> Teresa >> >>> >>> 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 >> >> >> >> -- >> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-20 4:57 ` Teresa Johnson @ 2015-03-20 5:38 ` Xinliang David Li 2015-03-20 15:15 ` Teresa Johnson 0 siblings, 1 reply; 10+ messages in thread From: Xinliang David Li @ 2015-03-20 5:38 UTC (permalink / raw) To: Teresa Johnson; +Cc: gcc-patches On Thu, Mar 19, 2015 at 9:57 PM, Teresa Johnson <tejohnson@google.com> wrote: > On Thu, Mar 19, 2015 at 8:00 PM, Xinliang David Li <davidxl@google.com> wrote: >> ok -- then there is an over assertion in get_local_tls_init_fn. The >> method can be called for aux module -- the decl created will also be >> promoted. > > No, it won't ever be called for an aux module. Both callsites are > guarded (handle_tls_init has an early return at the top, and the > callsite is guarded in get_tls_iniit_fn). > >> >> Also can we rely on late promotion (special case of artificial >> function __tls_init? This can avoid duplicated logic here. > > We don't need to promote __tls_init, but rather the init functions for > each TLS variable (which are each aliased to __tls_init). > I thought > about both approaches, but promoting it as it was created seemed more > straightforward. I can give that approach a try though if you prefer > it (special casing DECL_ARTIFICIAL functions that start with _ZTH). > Incidentally they are not marked static, so that would also need to be > done to get them promoted. For public tls symbols in aux module, tls wrappers will reference _ZTHxx init functions which are aliases to __tls_init. If __tls_init is not created for aux modules, what symbol will those _ZTHxx funcitons aliased to? Is it better just let _tls_init function to be created as usual, but let the standard promotion kick in later? The ZTHxx functions can still be marked as alias to the promoted __tls_init? David > > Teresa > >> >> David >> >> On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: >>>> does generate_tls_wrapper also need to be suppressed for aux module? >>> >>> No, we generate the wrapper in the aux module and use it to access the >>> TLS variable and invoke it's init function (which is defined in the >>> variable's own module). Presumably we could generate that only in the >>> original module and promote it if necessary, but generating it in the >>> aux module does allow it to be inlined. This is essentially how the >>> TLS accesses are handled for extern TLS variables defined elsewhere >>> (wrapper generated in referring module). >>> >>> Teresa >>> >>>> >>>> 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 >>> >>> >>> >>> -- >>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-20 5:38 ` Xinliang David Li @ 2015-03-20 15:15 ` Teresa Johnson 2015-03-20 20:40 ` Teresa Johnson 0 siblings, 1 reply; 10+ messages in thread From: Teresa Johnson @ 2015-03-20 15:15 UTC (permalink / raw) To: Xinliang David Li; +Cc: gcc-patches On Thu, Mar 19, 2015 at 10:38 PM, Xinliang David Li <davidxl@google.com> wrote: > On Thu, Mar 19, 2015 at 9:57 PM, Teresa Johnson <tejohnson@google.com> wrote: >> On Thu, Mar 19, 2015 at 8:00 PM, Xinliang David Li <davidxl@google.com> wrote: >>> ok -- then there is an over assertion in get_local_tls_init_fn. The >>> method can be called for aux module -- the decl created will also be >>> promoted. >> >> No, it won't ever be called for an aux module. Both callsites are >> guarded (handle_tls_init has an early return at the top, and the >> callsite is guarded in get_tls_iniit_fn). >> >>> >>> Also can we rely on late promotion (special case of artificial >>> function __tls_init? This can avoid duplicated logic here. >> >> We don't need to promote __tls_init, but rather the init functions for >> each TLS variable (which are each aliased to __tls_init). > >> I thought >> about both approaches, but promoting it as it was created seemed more >> straightforward. I can give that approach a try though if you prefer >> it (special casing DECL_ARTIFICIAL functions that start with _ZTH). >> Incidentally they are not marked static, so that would also need to be >> done to get them promoted. > > For public tls symbols in aux module, tls wrappers will reference > _ZTHxx init functions which are aliases to __tls_init. If __tls_init > is not created for aux modules, what symbol will those _ZTHxx > funcitons aliased to? > > Is it better just let _tls_init function to be created as usual, but > let the standard promotion kick in later? The ZTHxx functions can > still be marked as alias to the promoted __tls_init? So there are 3 function decls being generated for TLS variables: 1) A single __tls_init function per module This initializes *all* TLS variables in the module, guarded by a single __tls_guard variable to ensure it is only done once. 2) One _ZTHxx init function per TLS variable Each of these is aliased to the module's __tls_init 3) One _ZTWxx wrapper function per TLS variable This is called in place of accesses to that TLS variable. It invokes the _ZTHxx init function for the variable to initialize it if necessary. I am concerned about generating the aux module __tls_init functions and then promoting them as they contain initializations for all TLS variables from the aux module - generating it each time the module is included as an aux module seems inefficient at best. What happens in the case where the TLS variable is declared extern and accessed by a separate module (and essentially is what happens with my patch when it is file static, promoted and accessed via an aux module) is this: The _ZTHxx init functions are aliased to __tls_init in the module containing the definition, and are externally-visible (after promoting in the file-static exported primary module case). The external module containing the reference (or the primary module that imported the defining module as an aux module in the LIPO case) generates a _ZTWxx wrapper for the TLS variable, which references its _ZTHxx init function. This ends up resolved to the exported/promoted _ZTHxx symbol in the defining module, which in turn is aliased to the defining module's __tls_init function. Teresa > > David > > > > >> >> Teresa >> >>> >>> David >>> >>> On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>> On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: >>>>> does generate_tls_wrapper also need to be suppressed for aux module? >>>> >>>> No, we generate the wrapper in the aux module and use it to access the >>>> TLS variable and invoke it's init function (which is defined in the >>>> variable's own module). Presumably we could generate that only in the >>>> original module and promote it if necessary, but generating it in the >>>> aux module does allow it to be inlined. This is essentially how the >>>> TLS accesses are handled for extern TLS variables defined elsewhere >>>> (wrapper generated in referring module). >>>> >>>> Teresa >>>> >>>>> >>>>> 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 >>>> >>>> >>>> >>>> -- >>>> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-20 15:15 ` Teresa Johnson @ 2015-03-20 20:40 ` Teresa Johnson 2015-03-20 20:50 ` Xinliang David Li 0 siblings, 1 reply; 10+ messages in thread From: Teresa Johnson @ 2015-03-20 20:40 UTC (permalink / raw) To: Xinliang David Li; +Cc: gcc-patches After offline discussions with David, I changed to the approach of generating __tls_init for the aux module, but then modifying the LIPO code to allow it to be statically promoted and treated as an aux function as a special case of an artificial decl. That works well. New patch below. Fixed the tests to validate rather than print the expected output. Ok for google branches? 2015-03-20 Teresa Johnson <tejohnson@google.com> gcc/ Google ref b/19618364. * cp/decl2.c (get_local_tls_init_fn): Mark static, record new global at module scope. (get_tls_init_fn): Record new global at module scope. (get_tls_wrapper_fn): Ditto. (handle_tls_init): Suppress alias in aux module. * l-ipo.c (decl_artificial_nopromote): New function. (cgraph_is_aux_decl_external): Call new function. (process_module_scope_static_func): Ditto. 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) @@ -3033,9 +3033,15 @@ get_local_tls_init_fn (void) void_list_node)); SET_DECL_LANGUAGE (fn, lang_c); TREE_PUBLIC (fn) = false; + TREE_STATIC (fn) = true; DECL_ARTIFICIAL (fn) = true; mark_used (fn); 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; } @@ -3100,6 +3106,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 +3168,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; } @@ -4213,8 +4229,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. Don't emit + aliases in LIPO aux modules, since the corresponding __tls_init + will be static promoted and deleted, so the variable's tls init + function will be resolved by its own primary module. An alias + would prevent the promoted aux __tls_init from being deleted. */ + if (TREE_PUBLIC (var) && !L_IPO_IS_AUXILIARY_MODULE) { tree single_init_fn = get_tls_init_fn (var); if (single_init_fn == NULL_TREE) Index: l-ipo.c =================================================================== --- l-ipo.c (revision 221425) +++ l-ipo.c (working copy) @@ -1120,6 +1120,27 @@ cgraph_unify_type_alias_sets (void) htab_delete (type_hash_tab); } +/* Return true if DECL is an artificial function that we do not want + to promote and which may not be available in the primary module. + The sole exception is currently __tls_init. */ + +static bool +decl_artificial_nopromote (tree decl) +{ + if (!DECL_ARTIFICIAL (decl)) + return false; + + /* Handle the __tls_init function specially as we do want to promote it and + allow the aux module to be resolved to the version in the primary module. + We check if it is prefixed by __tls_init to catch it after promotion + as well from cgraph_is_aux_decl_external. */ + if (!strncmp (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), "__tls_init", + 10)) + return false; + + return true; +} + /* Return true if NODE->decl from an auxiliary module has external definition (and therefore is not needed for expansion). */ @@ -1144,7 +1165,7 @@ cgraph_is_aux_decl_external (struct cgraph_node *n Functions marked artificial (e.g. an implicitly instantiated virtual destructor) are also not guaranteed to be available in the primary module, as they are not promoted by process_module_scope_static_func. */ - if (DECL_COMDAT (decl) || DECL_WEAK (decl) || DECL_ARTIFICIAL (decl)) + if (DECL_COMDAT (decl) || DECL_WEAK (decl) || decl_artificial_nopromote (decl)) return false; /* virtual functions won't be deleted in the primary module. */ @@ -2022,7 +2043,7 @@ process_module_scope_static_func (struct cgraph_no if (TREE_PUBLIC (decl) || !TREE_STATIC (decl) || DECL_EXTERNAL (decl) - || DECL_ARTIFICIAL (decl)) + || decl_artificial_nopromote (decl)) return; if (flag_ripa_no_promote_always_inline 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,31 @@ +// { 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() { + int i = 0; + if (GetTLSClass()->id != i++) + abort(); + TLSClass *A = NextTLSClass(); + SetTLSClass(A); + if (GetTLSClass()->id != i++) + abort(); + if (current_tls2_->id != i++) + abort(); + A = NextTLSClass(); + SetTLSClass2(A); + if (current_tls2_->id != i++) + abort(); +} 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,38 @@ +// { 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() { + int i = 0; + if (GetTLSClass()->id != i++) + abort(); + TLSClass *A = NextTLSClass(); + SetTLSClass(A); + if (GetTLSClass()->id != i++) + abort(); + A = NextTLSClass(); + SetTLSClassHere(A); + if (GetTLSClass()->id != i++) + abort(); + if (current_tls2_->id != i++) + abort(); + A = NextTLSClass(); + SetTLSClass2(A); + if (current_tls2_->id != i++) + abort(); +} On Fri, Mar 20, 2015 at 8:15 AM, Teresa Johnson <tejohnson@google.com> wrote: > On Thu, Mar 19, 2015 at 10:38 PM, Xinliang David Li <davidxl@google.com> wrote: >> On Thu, Mar 19, 2015 at 9:57 PM, Teresa Johnson <tejohnson@google.com> wrote: >>> On Thu, Mar 19, 2015 at 8:00 PM, Xinliang David Li <davidxl@google.com> wrote: >>>> ok -- then there is an over assertion in get_local_tls_init_fn. The >>>> method can be called for aux module -- the decl created will also be >>>> promoted. >>> >>> No, it won't ever be called for an aux module. Both callsites are >>> guarded (handle_tls_init has an early return at the top, and the >>> callsite is guarded in get_tls_iniit_fn). >>> >>>> >>>> Also can we rely on late promotion (special case of artificial >>>> function __tls_init? This can avoid duplicated logic here. >>> >>> We don't need to promote __tls_init, but rather the init functions for >>> each TLS variable (which are each aliased to __tls_init). >> >>> I thought >>> about both approaches, but promoting it as it was created seemed more >>> straightforward. I can give that approach a try though if you prefer >>> it (special casing DECL_ARTIFICIAL functions that start with _ZTH). >>> Incidentally they are not marked static, so that would also need to be >>> done to get them promoted. >> >> For public tls symbols in aux module, tls wrappers will reference >> _ZTHxx init functions which are aliases to __tls_init. If __tls_init >> is not created for aux modules, what symbol will those _ZTHxx >> funcitons aliased to? >> >> Is it better just let _tls_init function to be created as usual, but >> let the standard promotion kick in later? The ZTHxx functions can >> still be marked as alias to the promoted __tls_init? > > So there are 3 function decls being generated for TLS variables: > > 1) A single __tls_init function per module > This initializes *all* TLS variables in the module, guarded by a > single __tls_guard variable to ensure it is only done once. > > 2) One _ZTHxx init function per TLS variable > Each of these is aliased to the module's __tls_init > > 3) One _ZTWxx wrapper function per TLS variable > This is called in place of accesses to that TLS variable. It invokes > the _ZTHxx init function for the variable to initialize it if > necessary. > > I am concerned about generating the aux module __tls_init functions > and then promoting them as they contain initializations for all TLS > variables from the aux module - generating it each time the module is > included as an aux module seems inefficient at best. > > What happens in the case where the TLS variable is declared extern and > accessed by a separate module (and essentially is what happens with my > patch when it is file static, promoted and accessed via an aux module) > is this: The _ZTHxx init functions are aliased to __tls_init in the > module containing the definition, and are externally-visible (after > promoting in the file-static exported primary module case). The > external module containing the reference (or the primary module that > imported the defining module as an aux module in the LIPO case) > generates a _ZTWxx wrapper for the TLS variable, which references its > _ZTHxx init function. This ends up resolved to the exported/promoted > _ZTHxx symbol in the defining module, which in turn is aliased to the > defining module's __tls_init function. > > Teresa > >> >> David >> >> >> >> >>> >>> Teresa >>> >>>> >>>> David >>>> >>>> On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>>> On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: >>>>>> does generate_tls_wrapper also need to be suppressed for aux module? >>>>> >>>>> No, we generate the wrapper in the aux module and use it to access the >>>>> TLS variable and invoke it's init function (which is defined in the >>>>> variable's own module). Presumably we could generate that only in the >>>>> original module and promote it if necessary, but generating it in the >>>>> aux module does allow it to be inlined. This is essentially how the >>>>> TLS accesses are handled for extern TLS variables defined elsewhere >>>>> (wrapper generated in referring module). >>>>> >>>>> Teresa >>>>> >>>>>> >>>>>> 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 >>>>> >>>>> >>>>> >>>>> -- >>>>> 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 -- Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode 2015-03-20 20:40 ` Teresa Johnson @ 2015-03-20 20:50 ` Xinliang David Li 0 siblings, 0 replies; 10+ messages in thread From: Xinliang David Li @ 2015-03-20 20:50 UTC (permalink / raw) To: Teresa Johnson; +Cc: gcc-patches ok thanks. David On Fri, Mar 20, 2015 at 1:40 PM, Teresa Johnson <tejohnson@google.com> wrote: > After offline discussions with David, I changed to the approach of > generating __tls_init for the aux module, but then modifying the LIPO > code to allow it to be statically promoted and treated as an aux > function as a special case of an artificial decl. That works well. New > patch below. Fixed the tests to validate rather than print the > expected output. > > Ok for google branches? > > 2015-03-20 Teresa Johnson <tejohnson@google.com> > > gcc/ > Google ref b/19618364. > * cp/decl2.c (get_local_tls_init_fn): Mark static, > record new global at module scope. > (get_tls_init_fn): Record new global at module scope. > (get_tls_wrapper_fn): Ditto. > (handle_tls_init): Suppress alias in aux module. > * l-ipo.c (decl_artificial_nopromote): New function. > (cgraph_is_aux_decl_external): Call new function. > (process_module_scope_static_func): Ditto. > > 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) > @@ -3033,9 +3033,15 @@ get_local_tls_init_fn (void) > void_list_node)); > SET_DECL_LANGUAGE (fn, lang_c); > TREE_PUBLIC (fn) = false; > + TREE_STATIC (fn) = true; > DECL_ARTIFICIAL (fn) = true; > mark_used (fn); > 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; > } > @@ -3100,6 +3106,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 +3168,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; > } > @@ -4213,8 +4229,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. Don't emit > + aliases in LIPO aux modules, since the corresponding __tls_init > + will be static promoted and deleted, so the variable's tls init > + function will be resolved by its own primary module. An alias > + would prevent the promoted aux __tls_init from being deleted. */ > + if (TREE_PUBLIC (var) && !L_IPO_IS_AUXILIARY_MODULE) > { > tree single_init_fn = get_tls_init_fn (var); > if (single_init_fn == NULL_TREE) > Index: l-ipo.c > =================================================================== > --- l-ipo.c (revision 221425) > +++ l-ipo.c (working copy) > @@ -1120,6 +1120,27 @@ cgraph_unify_type_alias_sets (void) > htab_delete (type_hash_tab); > } > > +/* Return true if DECL is an artificial function that we do not want > + to promote and which may not be available in the primary module. > + The sole exception is currently __tls_init. */ > + > +static bool > +decl_artificial_nopromote (tree decl) > +{ > + if (!DECL_ARTIFICIAL (decl)) > + return false; > + > + /* Handle the __tls_init function specially as we do want to promote it and > + allow the aux module to be resolved to the version in the primary module. > + We check if it is prefixed by __tls_init to catch it after promotion > + as well from cgraph_is_aux_decl_external. */ > + if (!strncmp (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), "__tls_init", > + 10)) > + return false; > + > + return true; > +} > + > /* Return true if NODE->decl from an auxiliary module has external > definition (and therefore is not needed for expansion). */ > > @@ -1144,7 +1165,7 @@ cgraph_is_aux_decl_external (struct cgraph_node *n > Functions marked artificial (e.g. an implicitly instantiated virtual > destructor) are also not guaranteed to be available in the primary module, > as they are not promoted by process_module_scope_static_func. */ > - if (DECL_COMDAT (decl) || DECL_WEAK (decl) || DECL_ARTIFICIAL (decl)) > + if (DECL_COMDAT (decl) || DECL_WEAK (decl) || > decl_artificial_nopromote (decl)) > return false; > > /* virtual functions won't be deleted in the primary module. */ > @@ -2022,7 +2043,7 @@ process_module_scope_static_func (struct cgraph_no > if (TREE_PUBLIC (decl) > || !TREE_STATIC (decl) > || DECL_EXTERNAL (decl) > - || DECL_ARTIFICIAL (decl)) > + || decl_artificial_nopromote (decl)) > return; > > if (flag_ripa_no_promote_always_inline > 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,31 @@ > +// { 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() { > + int i = 0; > + if (GetTLSClass()->id != i++) > + abort(); > + TLSClass *A = NextTLSClass(); > + SetTLSClass(A); > + if (GetTLSClass()->id != i++) > + abort(); > + if (current_tls2_->id != i++) > + abort(); > + A = NextTLSClass(); > + SetTLSClass2(A); > + if (current_tls2_->id != i++) > + abort(); > +} > 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,38 @@ > +// { 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() { > + int i = 0; > + if (GetTLSClass()->id != i++) > + abort(); > + TLSClass *A = NextTLSClass(); > + SetTLSClass(A); > + if (GetTLSClass()->id != i++) > + abort(); > + A = NextTLSClass(); > + SetTLSClassHere(A); > + if (GetTLSClass()->id != i++) > + abort(); > + if (current_tls2_->id != i++) > + abort(); > + A = NextTLSClass(); > + SetTLSClass2(A); > + if (current_tls2_->id != i++) > + abort(); > +} > > On Fri, Mar 20, 2015 at 8:15 AM, Teresa Johnson <tejohnson@google.com> wrote: >> On Thu, Mar 19, 2015 at 10:38 PM, Xinliang David Li <davidxl@google.com> wrote: >>> On Thu, Mar 19, 2015 at 9:57 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>> On Thu, Mar 19, 2015 at 8:00 PM, Xinliang David Li <davidxl@google.com> wrote: >>>>> ok -- then there is an over assertion in get_local_tls_init_fn. The >>>>> method can be called for aux module -- the decl created will also be >>>>> promoted. >>>> >>>> No, it won't ever be called for an aux module. Both callsites are >>>> guarded (handle_tls_init has an early return at the top, and the >>>> callsite is guarded in get_tls_iniit_fn). >>>> >>>>> >>>>> Also can we rely on late promotion (special case of artificial >>>>> function __tls_init? This can avoid duplicated logic here. >>>> >>>> We don't need to promote __tls_init, but rather the init functions for >>>> each TLS variable (which are each aliased to __tls_init). >>> >>>> I thought >>>> about both approaches, but promoting it as it was created seemed more >>>> straightforward. I can give that approach a try though if you prefer >>>> it (special casing DECL_ARTIFICIAL functions that start with _ZTH). >>>> Incidentally they are not marked static, so that would also need to be >>>> done to get them promoted. >>> >>> For public tls symbols in aux module, tls wrappers will reference >>> _ZTHxx init functions which are aliases to __tls_init. If __tls_init >>> is not created for aux modules, what symbol will those _ZTHxx >>> funcitons aliased to? >>> >>> Is it better just let _tls_init function to be created as usual, but >>> let the standard promotion kick in later? The ZTHxx functions can >>> still be marked as alias to the promoted __tls_init? >> >> So there are 3 function decls being generated for TLS variables: >> >> 1) A single __tls_init function per module >> This initializes *all* TLS variables in the module, guarded by a >> single __tls_guard variable to ensure it is only done once. >> >> 2) One _ZTHxx init function per TLS variable >> Each of these is aliased to the module's __tls_init >> >> 3) One _ZTWxx wrapper function per TLS variable >> This is called in place of accesses to that TLS variable. It invokes >> the _ZTHxx init function for the variable to initialize it if >> necessary. >> >> I am concerned about generating the aux module __tls_init functions >> and then promoting them as they contain initializations for all TLS >> variables from the aux module - generating it each time the module is >> included as an aux module seems inefficient at best. >> >> What happens in the case where the TLS variable is declared extern and >> accessed by a separate module (and essentially is what happens with my >> patch when it is file static, promoted and accessed via an aux module) >> is this: The _ZTHxx init functions are aliased to __tls_init in the >> module containing the definition, and are externally-visible (after >> promoting in the file-static exported primary module case). The >> external module containing the reference (or the primary module that >> imported the defining module as an aux module in the LIPO case) >> generates a _ZTWxx wrapper for the TLS variable, which references its >> _ZTHxx init function. This ends up resolved to the exported/promoted >> _ZTHxx symbol in the defining module, which in turn is aliased to the >> defining module's __tls_init function. >> >> Teresa >> >>> >>> David >>> >>> >>> >>> >>>> >>>> Teresa >>>> >>>>> >>>>> David >>>>> >>>>> On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson <tejohnson@google.com> wrote: >>>>>> On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li <davidxl@google.com> wrote: >>>>>>> does generate_tls_wrapper also need to be suppressed for aux module? >>>>>> >>>>>> No, we generate the wrapper in the aux module and use it to access the >>>>>> TLS variable and invoke it's init function (which is defined in the >>>>>> variable's own module). Presumably we could generate that only in the >>>>>> original module and promote it if necessary, but generating it in the >>>>>> aux module does allow it to be inlined. This is essentially how the >>>>>> TLS accesses are handled for extern TLS variables defined elsewhere >>>>>> (wrapper generated in referring module). >>>>>> >>>>>> Teresa >>>>>> >>>>>>> >>>>>>> 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 >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> 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 > > > > -- > Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-03-20 20:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CAAe5K+WnYun4Jkcq=tgjTz_iXaYeLBa8w-NLDji37zv6WSu_mg@mail.gmail.com>
[not found] ` <CAAkRFZJrvb2X+s=BJpHwaCrBuzvUCNh5GAB0fGZS_VbvPOveyw@mail.gmail.com>
2015-03-19 13:09 ` [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode Teresa Johnson
2015-03-19 23:11 ` Teresa Johnson
2015-03-19 23:46 ` Xinliang David Li
2015-03-20 0:51 ` Teresa Johnson
2015-03-20 3:01 ` Xinliang David Li
2015-03-20 4:57 ` Teresa Johnson
2015-03-20 5:38 ` Xinliang David Li
2015-03-20 15:15 ` Teresa Johnson
2015-03-20 20:40 ` Teresa Johnson
2015-03-20 20:50 ` Xinliang David Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).