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: Teresa Johnson <tejohnson at google dot com>
- To: Xinliang David Li <davidxl 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 16:11:37 -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>
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