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