From patchwork Fri Mar 20 20:40:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teresa Johnson X-Patchwork-Id: 452814 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 360A614008F for ; Sat, 21 Mar 2015 07:40:36 +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=cjfTd31D; 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=StCOWo7RXdZCvfWlTy qNoGqoatvmdWUelscJoEJf/3ORoE0HI1oeG+G7UFllaJBlhRUC24w8byPKimfZb1 KEjxhnG4WVOvSWEVpWSZ3P0clO2AUZSS76VtAiDUn7Sh/URf2wYjDT7gquiYZLHB mcYzCkhYMEi8KiP508sy8MkJI= 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=vVQWZaFT8D0kJnKjeKj0AG7E dys=; b=cjfTd31DPmPXfljWoLFC/FgdY4yNlF3ffAs0E232p8x5DufmNZ6Bh2mm Y+Y3f13F1tX1woO+9rlSTMczwJ1CNHTkCoedOWTZwvJsgAQJn83xnKIrjDxlgfmp 4SO9I/pcw78NxTJE7Opf1UMvFh+tPAVhon+M749a1Xc3uDk9Oog= Received: (qmail 97804 invoked by alias); 20 Mar 2015 20:40:27 -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 97623 invoked by uid 89); 20 Mar 2015 20:40:25 -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-ob0-f169.google.com Received: from mail-ob0-f169.google.com (HELO mail-ob0-f169.google.com) (209.85.214.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 20 Mar 2015 20:40:20 +0000 Received: by obbgg8 with SMTP id gg8so86545531obb.1 for ; Fri, 20 Mar 2015 13:40:18 -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=BmpL7bRkelCpAFNu+EWyepz+9RgOrgSCM1G8SI09EiQ=; b=IO50MMnnHn/2iwygQUpby20GqSySPFTgk/7hzq8FQDd87veUZ8cu/KfKp+ZwtdoN/O QlODofYcyyfZE+8iTAy84V/s5wIiamj/zT0aic4k8X7yMSSsCUpKI6IEKDLWuLwVOSYZ 9N5SWwHhcK2EV5DkvWViyr6jqHktHJtKdAU7U8e1H4/avjdFp5wd2Bz8zIM2mto1MUea sNkt/NG3apwyD+vujZQ6WuemltyJ5VkUOIfNMJS7KDtQf3599vRmRv74DfJ8JG9cPNxR eYwR7zZiBjG+Pu+3+HHkLaZKySJT8eNdFkecxYNoYguB3L/YboI6/pibAOrFg2tBgzct +VkQ== X-Gm-Message-State: ALoCoQnSvadPohC/ie4ApKJxIkK+HIRocrIArTmiLLiLJy1evu48eWhmWuuFqku12jXXDVVo9Wle MIME-Version: 1.0 X-Received: by 10.202.84.16 with SMTP id i16mr36036407oib.129.1426884018254; Fri, 20 Mar 2015 13:40:18 -0700 (PDT) Received: by 10.76.25.98 with HTTP; Fri, 20 Mar 2015 13:40:18 -0700 (PDT) In-Reply-To: References: Date: Fri, 20 Mar 2015 13:40:18 -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 After offline discussions with David, I changed to the approach of generating __tls_init for the aux module, but then modifying the LIPO code to allow it to be statically promoted and treated as an aux function as a special case of an artificial decl. That works well. New patch below. Fixed the tests to validate rather than print the expected output. Ok for google branches? 2015-03-20 Teresa Johnson gcc/ Google ref b/19618364. * cp/decl2.c (get_local_tls_init_fn): Mark static, record new global at module scope. (get_tls_init_fn): Record new global at module scope. (get_tls_wrapper_fn): Ditto. (handle_tls_init): Suppress alias in aux module. * l-ipo.c (decl_artificial_nopromote): New function. (cgraph_is_aux_decl_external): Call new function. (process_module_scope_static_func): Ditto. 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 Fri, Mar 20, 2015 at 8:15 AM, Teresa Johnson wrote: > On Thu, Mar 19, 2015 at 10:38 PM, Xinliang David Li wrote: >> On Thu, Mar 19, 2015 at 9:57 PM, Teresa Johnson wrote: >>> On Thu, Mar 19, 2015 at 8:00 PM, Xinliang David Li 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? > > So there are 3 function decls being generated for TLS variables: > > 1) A single __tls_init function per module > This initializes *all* TLS variables in the module, guarded by a > single __tls_guard variable to ensure it is only done once. > > 2) One _ZTHxx init function per TLS variable > Each of these is aliased to the module's __tls_init > > 3) One _ZTWxx wrapper function per TLS variable > This is called in place of accesses to that TLS variable. It invokes > the _ZTHxx init function for the variable to initialize it if > necessary. > > I am concerned about generating the aux module __tls_init functions > and then promoting them as they contain initializations for all TLS > variables from the aux module - generating it each time the module is > included as an aux module seems inefficient at best. > > What happens in the case where the TLS variable is declared extern and > accessed by a separate module (and essentially is what happens with my > patch when it is file static, promoted and accessed via an aux module) > is this: The _ZTHxx init functions are aliased to __tls_init in the > module containing the definition, and are externally-visible (after > promoting in the file-static exported primary module case). The > external module containing the reference (or the primary module that > imported the defining module as an aux module in the LIPO case) > generates a _ZTWxx wrapper for the TLS variable, which references its > _ZTHxx init function. This ends up resolved to the exported/promoted > _ZTHxx symbol in the defining module, which in turn is aliased to the > defining module's __tls_init function. > > Teresa > >> >> David >> >> >> >> >>> >>> Teresa >>> >>>> >>>> David >>>> >>>> On Thu, Mar 19, 2015 at 5:51 PM, Teresa Johnson wrote: >>>>> On Thu, Mar 19, 2015 at 4:45 PM, Xinliang David Li 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 wrote: >>>>>>> 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. >>>>>>> >>>>>>> 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); >>>>>>> +} >>>>>>> >>>>>>> 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 >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> 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 Index: cp/decl2.c =================================================================== --- cp/decl2.c (revision 221425) +++ cp/decl2.c (working copy) @@ -3033,9 +3033,15 @@ get_local_tls_init_fn (void) void_list_node)); SET_DECL_LANGUAGE (fn, lang_c); TREE_PUBLIC (fn) = false; + TREE_STATIC (fn) = true; DECL_ARTIFICIAL (fn) = true; mark_used (fn); 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; } @@ -3100,6 +3106,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 +3168,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; } @@ -4213,8 +4229,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. Don't emit + aliases in LIPO aux modules, since the corresponding __tls_init + will be static promoted and deleted, so the variable's tls init + function will be resolved by its own primary module. An alias + would prevent the promoted aux __tls_init from being deleted. */ + if (TREE_PUBLIC (var) && !L_IPO_IS_AUXILIARY_MODULE) { tree single_init_fn = get_tls_init_fn (var); if (single_init_fn == NULL_TREE) Index: l-ipo.c =================================================================== --- l-ipo.c (revision 221425) +++ l-ipo.c (working copy) @@ -1120,6 +1120,27 @@ cgraph_unify_type_alias_sets (void) htab_delete (type_hash_tab); } +/* Return true if DECL is an artificial function that we do not want + to promote and which may not be available in the primary module. + The sole exception is currently __tls_init. */ + +static bool +decl_artificial_nopromote (tree decl) +{ + if (!DECL_ARTIFICIAL (decl)) + return false; + + /* Handle the __tls_init function specially as we do want to promote it and + allow the aux module to be resolved to the version in the primary module. + We check if it is prefixed by __tls_init to catch it after promotion + as well from cgraph_is_aux_decl_external. */ + if (!strncmp (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)), "__tls_init", + 10)) + return false; + + return true; +} + /* Return true if NODE->decl from an auxiliary module has external definition (and therefore is not needed for expansion). */ @@ -1144,7 +1165,7 @@ cgraph_is_aux_decl_external (struct cgraph_node *n Functions marked artificial (e.g. an implicitly instantiated virtual destructor) are also not guaranteed to be available in the primary module, as they are not promoted by process_module_scope_static_func. */ - if (DECL_COMDAT (decl) || DECL_WEAK (decl) || DECL_ARTIFICIAL (decl)) + if (DECL_COMDAT (decl) || DECL_WEAK (decl) || decl_artificial_nopromote (decl)) return false; /* virtual functions won't be deleted in the primary module. */ @@ -2022,7 +2043,7 @@ process_module_scope_static_func (struct cgraph_no if (TREE_PUBLIC (decl) || !TREE_STATIC (decl) || DECL_EXTERNAL (decl) - || DECL_ARTIFICIAL (decl)) + || decl_artificial_nopromote (decl)) return; if (flag_ripa_no_promote_always_inline 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,31 @@ +// { 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() { + int i = 0; + if (GetTLSClass()->id != i++) + abort(); + TLSClass *A = NextTLSClass(); + SetTLSClass(A); + if (GetTLSClass()->id != i++) + abort(); + if (current_tls2_->id != i++) + abort(); + A = NextTLSClass(); + SetTLSClass2(A); + if (current_tls2_->id != i++) + abort(); +} 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,38 @@ +// { 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() { + int i = 0; + if (GetTLSClass()->id != i++) + abort(); + TLSClass *A = NextTLSClass(); + SetTLSClass(A); + if (GetTLSClass()->id != i++) + abort(); + A = NextTLSClass(); + SetTLSClassHere(A); + if (GetTLSClass()->id != i++) + abort(); + if (current_tls2_->id != i++) + abort(); + A = NextTLSClass(); + SetTLSClass2(A); + if (current_tls2_->id != i++) + abort(); +}