[GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode
Teresa Johnson
tejohnson@google.com
Fri Mar 20 00:51:00 GMT 2015
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
More information about the Gcc-patches
mailing list