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?
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
