This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode
- From: Xinliang David Li <davidxl at google dot com>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 19 Mar 2015 20:00:59 -0700
- Subject: Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode
- Authentication-results: sourceware.org; auth=none
- References: <CAAe5K+WnYun4Jkcq=tgjTz_iXaYeLBa8w-NLDji37zv6WSu_mg at mail dot gmail dot com> <CAAkRFZJrvb2X+s=BJpHwaCrBuzvUCNh5GAB0fGZS_VbvPOveyw at mail dot gmail dot com> <CAAe5K+WNCaD6BxTmnTp3P44jxrkyQN6Qj1ooPLU4qwMewgRu4w at mail dot gmail dot com> <CAAe5K+VqS_EtXX2pX9JE0E7kEh-+H6AzrCKN8Do9eYwTkskqJQ at mail dot gmail dot com> <CAAkRFZKC-s1TiLMOYmU3vQ3bR7=OXZLENHbz8165FA2Z1he0Cw at mail dot gmail dot com> <CAAe5K+XjXr0otnHD+Z8tWFkMMPK1s6AyMyODHFrBnmrnh2Rd7w at mail dot gmail dot com>
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