From patchwork Thu Mar 19 23:11:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teresa Johnson X-Patchwork-Id: 452395 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 69B6B140079 for ; Fri, 20 Mar 2015 10:11:49 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass reason="1024-bit key; unprotected key" header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=YGcwDWFG; dkim-adsp=none (unprotected policy); dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=HZURiKH1tbL14e5W8g GYukhS3uFOIoD3Z/UNx781lIZ4qhUrn0zlXwJTqj+P2udy0k4NczOkIusfWwTeeT Ktt4KzpBj1xYM8atsieNVTqxb1sadMwpOjg/owEIlCIqZ5z55e48ZbAwGNYJAXPm Ga80o0gtQdZ6h05OjAll7YM38= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=lxFr8XfWsnY9FzduF+2rYZVB C6o=; b=YGcwDWFGi+nYmBTpDBYGV0p2w82x3qLllenWuDedRez1tMqlRA6+oxLy 19tbfqRlfw6WS2db5v7Vvu44rK3mzJjHQ5SE/C4Dk8bZe1jj3fWN94vcsKT6yCm+ zbarmvTMjE28LlXOiuJa15Q0xVlkwhgPYhDpfRxphTiiHPsjmno= Received: (qmail 43375 invoked by alias); 19 Mar 2015 23:11:42 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 43355 invoked by uid 89); 19 Mar 2015 23:11:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-oi0-f42.google.com Received: from mail-oi0-f42.google.com (HELO mail-oi0-f42.google.com) (209.85.218.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 19 Mar 2015 23:11:39 +0000 Received: by oifl3 with SMTP id l3so48043003oif.0 for ; Thu, 19 Mar 2015 16:11:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=U8rX+9rkwJPlw4V2XVy/f4//AAOmz9Vx+E5l5/YwlOo=; b=Lsfr6zO6Gs8R6FfzAcMdAkJbAexmGVNntGeZSesrjJgrq4K5J49egxaflWTGsHqL+5 7tSiy32UivwmbrRfW1ilhYLVYWGDf+qrwcrw1Baf8Ye559f5li25WGzs3vZVhK6W41V8 cSynLuQkw27rFzNJpZTg3kVdky9CMyP9aeMnEKQkbephj/AultO5qaUp1ogZmTIKfHMI RqmwIbsTPlJpYbmeioWW809BT3OeEhHZjfkRYtnC2sM3v98dCSl1cCiql1kApa45TfEl +SMiAHdjwADq2iVqbZUkz8oDWkLjTFJUTZj6Xxw92r6tGV8ZFQ49GZRSY5XsrTEiyTQH RZqA== X-Gm-Message-State: ALoCoQk7AfozC+agQB/U79QTvDroHUn6Y35RsChUvEO/6D9mrDatIxB72MSFxKm/Zdk1UvWHMuVL MIME-Version: 1.0 X-Received: by 10.60.131.206 with SMTP id oo14mr54110482oeb.30.1426806697312; Thu, 19 Mar 2015 16:11:37 -0700 (PDT) Received: by 10.76.25.98 with HTTP; Thu, 19 Mar 2015 16:11:37 -0700 (PDT) In-Reply-To: References: Date: Thu, 19 Mar 2015 16:11:37 -0700 Message-ID: Subject: Re: [GOOGLE] Fixes for TLS wrapper and init functions in LIPO mode From: Teresa Johnson To: Xinliang David Li Cc: "gcc-patches@gcc.gnu.org" X-IsSubscribed: yes New patch below. Passes regression tests plus internal application build. 2015-03-19 Teresa Johnson 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. On Thu, Mar 19, 2015 at 6:09 AM, Teresa Johnson wrote: > On Wed, Mar 18, 2015 at 9:25 PM, Xinliang David Li 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 >> 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 >>> >>> 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 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 +#include +#include +#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 +#include +#include +#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); +}