From patchwork Sat Mar 5 14:37:55 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 392 Return-Path: Delivered-To: unknown Received: from imap.gmail.com (74.125.159.109) by localhost6.localdomain6 with IMAP4-SSL; 08 Jun 2011 14:42:08 -0000 Delivered-To: patches@linaro.org Received: by 10.224.60.68 with SMTP id o4cs48116qah; Sat, 5 Mar 2011 06:38:07 -0800 (PST) Received: by 10.216.141.142 with SMTP id g14mr1714712wej.15.1299335886702; Sat, 05 Mar 2011 06:38:06 -0800 (PST) Received: from mail-wy0-f178.google.com (mail-wy0-f178.google.com [74.125.82.178]) by mx.google.com with ESMTPS id n47si894238wej.47.2011.03.05.06.38.05 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 05 Mar 2011 06:38:05 -0800 (PST) Received-SPF: pass (google.com: domain of rdsandiford@googlemail.com designates 74.125.82.178 as permitted sender) client-ip=74.125.82.178; Authentication-Results: mx.google.com; spf=pass (google.com: domain of rdsandiford@googlemail.com designates 74.125.82.178 as permitted sender) smtp.mail=rdsandiford@googlemail.com; dkim=pass (test mode) header.i=@googlemail.com Received: by wyf28 with SMTP id 28so3615269wyf.37 for ; Sat, 05 Mar 2011 06:38:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=gamma; h=domainkey-signature:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version:content-type; bh=cu8G/rCk5n5vZNikBDGTodKsjdRHw0DEDGoAZdp1jJE=; b=XHXgeM8ZcLYzFETQ+bjZOuWYKfZ2cWuTExyxpyQr78Am7uXi9CLojVEImlxYJtTu6V +879MhGjavd/oulnmehmekfP1hT/OsuM/6rOhFJeeQL9v620g3Yfm1xhQdQchIsH8+Am dUeVwrpEonidKZnt9EEhIJP8nYBK9wP4hu+YU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlemail.com; s=gamma; h=from:to:mail-followup-to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-type; b=txy1n5xGglVoKY+c5Q2c90/5dGt97wZ6EhG3dY4EM5gj7uSKC2f/eq09rTbX4WCRPz Hmc+yHjlr2437o8YRSvPJsuDHTgUJ8cV1Hu0GGGB/0y28Bql7C1KiOZbMDsT3H73nfo4 QZ2AYYdYVYZZ2ikTEIDtoZncFy1gPlRNdD/zE= Received: by 10.227.140.159 with SMTP id i31mr1549879wbu.166.1299335884748; Sat, 05 Mar 2011 06:38:04 -0800 (PST) Received: from localhost (rsandifo.gotadsl.co.uk [82.133.89.107]) by mx.google.com with ESMTPS id o6sm456334wbo.15.2011.03.05.06.38.02 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 05 Mar 2011 06:38:03 -0800 (PST) From: Richard Sandiford To: Jakub Jelinek Mail-Followup-To: Jakub Jelinek , gcc-patches@gcc.gnu.org, patches@linaro.org, richard.sandiford@linaro.org, rdsandiford@googlemail.com Cc: gcc-patches@gcc.gnu.org, patches@linaro.org, richard.sandiford@linaro.org Subject: Re: [2/2] Reducing the overhead of dwarf2 location tracking References: <20110304152205.GH30899@tyan-ft48-01.lab.bos.redhat.com> Date: Sat, 05 Mar 2011 14:37:55 +0000 In-Reply-To: <20110304152205.GH30899@tyan-ft48-01.lab.bos.redhat.com> (Jakub Jelinek's message of "Fri, 4 Mar 2011 16:22:05 +0100") Message-ID: <87mxl9vdj0.fsf@firetop.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Jakub Jelinek writes: > On Fri, Mar 04, 2011 at 01:56:55PM +0000, Richard Sandiford wrote: >> * dwarf2out.c (dw_loc_list_node): Add resolved_addr and replaced. >> (cached_dw_loc_list_def): New structure. >> (cached_dw_loc_list): New typedef. >> (cached_dw_loc_list_table): New variable. >> (cached_dw_loc_list_table_hash): New function. >> (cached_dw_loc_list_table_eq): Likewise. >> (add_location_or_const_value_attribute): Take a bool cache_p. >> Cache the list when the parameter is true. >> (gen_formal_parameter_die): Update caller. >> (gen_variable_die): Likewise. >> (dwarf2out_finish): Likewise. >> (dwarf2out_function_decl): Clear cached_dw_loc_list_table. >> (dwarf2out_init): Initialize cached_dw_loc_list_table. >> (resolve_addr): Cache the result of resolving a chain of >> location lists. > > I think you should handle the cached_dw_loc_list_table in > dwarf2out_abstract_function similarly to say decl_loc_table, i.e. > save/clear for the duration of the of recursive dwarf2out_decl > call, restore afterwards and in the places where you actually use > it guard it also with cached_dw_loc_list_table != NULL. OK, thanks for the pointer. How does this look? Bootstrapped & regression-tested on x86_64-linux-gnu. Richard gcc/ * dwarf2out.c (dw_loc_list_node): Add resolved_addr and replaced. (cached_dw_loc_list_def): New structure. (cached_dw_loc_list): New typedef. (cached_dw_loc_list_table): New variable. (cached_dw_loc_list_table_hash): New function. (cached_dw_loc_list_table_eq): Likewise. (add_location_or_const_value_attribute): Take a bool cache_p. Cache the list when the parameter is true. (gen_formal_parameter_die): Update caller. (gen_variable_die): Likewise. (dwarf2out_finish): Likewise. (dwarf2out_abstract_function): Nullify cached_dw_loc_list_table while generating debug info for the decl. (dwarf2out_function_decl): Clear cached_dw_loc_list_table. (dwarf2out_init): Initialize cached_dw_loc_list_table. (resolve_addr): Cache the result of resolving a chain of location lists. Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c 2011-03-05 09:02:43.000000000 +0000 +++ gcc/dwarf2out.c 2011-03-05 09:09:46.000000000 +0000 @@ -4464,6 +4464,11 @@ typedef struct GTY(()) dw_loc_list_struc const char *section; /* Section this loclist is relative to */ dw_loc_descr_ref expr; hashval_t hash; + /* True if all addresses in this and subsequent lists are known to be + resolved. */ + bool resolved_addr; + /* True if this list has been replaced by dw_loc_next. */ + bool replaced; bool emitted; } dw_loc_list_node; @@ -6119,6 +6124,19 @@ typedef struct var_loc_list_def var_loc_ /* Table of decl location linked lists. */ static GTY ((param_is (var_loc_list))) htab_t decl_loc_table; +/* A cached location list. */ +struct GTY (()) cached_dw_loc_list_def { + /* The DECL_UID of the decl that this entry describes. */ + unsigned int decl_id; + + /* The cached location list. */ + dw_loc_list_ref loc_list; +}; +typedef struct cached_dw_loc_list_def cached_dw_loc_list; + +/* Table of cached location lists. */ +static GTY ((param_is (cached_dw_loc_list))) htab_t cached_dw_loc_list_table; + /* A pointer to the base of a list of references to DIE's that are uniquely identified by their tag, presence/absence of children DIE's, and list of attribute/value pairs. */ @@ -6479,7 +6497,7 @@ static void insert_int (HOST_WIDE_INT, u static void insert_double (double_int, unsigned char *); static void insert_float (const_rtx, unsigned char *); static rtx rtl_for_decl_location (tree); -static bool add_location_or_const_value_attribute (dw_die_ref, tree, +static bool add_location_or_const_value_attribute (dw_die_ref, tree, bool, enum dwarf_attribute); static bool tree_add_const_value_attribute (dw_die_ref, tree); static bool tree_add_const_value_attribute_for_decl (dw_die_ref, tree); @@ -8201,6 +8219,24 @@ lookup_decl_loc (const_tree decl) htab_find_with_hash (decl_loc_table, decl, DECL_UID (decl)); } +/* Returns a hash value for X (which really is a cached_dw_loc_list_list). */ + +static hashval_t +cached_dw_loc_list_table_hash (const void *x) +{ + return (hashval_t) ((const cached_dw_loc_list *) x)->decl_id; +} + +/* Return nonzero if decl_id of cached_dw_loc_list X is the same as + UID of decl *Y. */ + +static int +cached_dw_loc_list_table_eq (const void *x, const void *y) +{ + return (((const cached_dw_loc_list *) x)->decl_id + == DECL_UID ((const_tree) y)); +} + /* Equate a DIE to a particular declaration. */ static void @@ -16978,15 +17014,22 @@ fortran_common (tree decl, HOST_WIDE_INT these things can crop up in other ways also.) Note that one type of constant value which can be passed into an inlined function is a constant pointer. This can happen for example if an actual argument in an inlined - function call evaluates to a compile-time constant address. */ + function call evaluates to a compile-time constant address. + + CACHE_P is true if it is worth caching the location list for DECL, + so that future calls can reuse it rather than regenerate it from scratch. + This is true for BLOCK_NONLOCALIZED_VARS in inlined subroutines, + since we will need to refer to them each time the function is inlined. */ static bool -add_location_or_const_value_attribute (dw_die_ref die, tree decl, +add_location_or_const_value_attribute (dw_die_ref die, tree decl, bool cache_p, enum dwarf_attribute attr) { rtx rtl; dw_loc_list_ref list; var_loc_list *loc_list; + cached_dw_loc_list *cache; + void **slot; if (TREE_CODE (decl) == ERROR_MARK) return false; @@ -17023,7 +17066,33 @@ add_location_or_const_value_attribute (d && add_const_value_attribute (die, rtl)) return true; } - list = loc_list_from_tree (decl, decl_by_reference_p (decl) ? 0 : 2); + /* If this decl is from BLOCK_NONLOCALIZED_VARS, we might need its + list several times. See if we've already cached the contents. */ + list = NULL; + if (loc_list == NULL || cached_dw_loc_list_table == NULL) + cache_p = false; + if (cache_p) + { + cache = (cached_dw_loc_list *) + htab_find_with_hash (cached_dw_loc_list_table, decl, DECL_UID (decl)); + if (cache) + list = cache->loc_list; + } + if (list == NULL) + { + list = loc_list_from_tree (decl, decl_by_reference_p (decl) ? 0 : 2); + /* It is usually worth caching this result if the decl is from + BLOCK_NONLOCALIZED_VARS and if the list has at least two elements. */ + if (cache_p && list && list->dw_loc_next) + { + slot = htab_find_slot_with_hash (cached_dw_loc_list_table, decl, + DECL_UID (decl), INSERT); + cache = ggc_alloc_cleared_cached_dw_loc_list (); + cache->decl_id = DECL_UID (decl); + cache->loc_list = list; + *slot = cache; + } + } if (list) { add_AT_location_description (die, attr, list); @@ -18684,7 +18753,7 @@ gen_formal_parameter_die (tree node, tre equate_decl_number_to_die (node, parm_die); if (! DECL_ABSTRACT (node_or_origin)) add_location_or_const_value_attribute (parm_die, node_or_origin, - DW_AT_location); + node == NULL, DW_AT_location); break; @@ -18869,6 +18938,7 @@ dwarf2out_abstract_function (tree decl) tree context; int was_abstract; htab_t old_decl_loc_table; + htab_t old_cached_dw_loc_list_table; /* Make sure we have the actual abstract inline, not a clone. */ decl = DECL_ORIGIN (decl); @@ -18882,7 +18952,9 @@ dwarf2out_abstract_function (tree decl) DIE. Be sure to not clobber the outer location table nor use it or we would get locations in abstract instantces. */ old_decl_loc_table = decl_loc_table; + old_cached_dw_loc_list_table = cached_dw_loc_list_table; decl_loc_table = NULL; + cached_dw_loc_list_table = NULL; /* Be sure we've emitted the in-class declaration DIE (if any) first, so we don't get confused by DECL_ABSTRACT. */ @@ -18907,6 +18979,7 @@ dwarf2out_abstract_function (tree decl) current_function_decl = save_fn; decl_loc_table = old_decl_loc_table; + cached_dw_loc_list_table = old_cached_dw_loc_list_table; pop_cfun (); } @@ -19723,9 +19796,8 @@ gen_variable_die (tree decl, tree origin && !TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (decl_or_origin))) defer_location (decl_or_origin, var_die); else - add_location_or_const_value_attribute (var_die, - decl_or_origin, - DW_AT_location); + add_location_or_const_value_attribute (var_die, decl_or_origin, + decl == NULL, DW_AT_location); add_pubname (decl_or_origin, var_die); } else @@ -21501,6 +21573,7 @@ dwarf2out_function_decl (tree decl) dwarf2out_decl (decl); htab_empty (decl_loc_table); + htab_empty (cached_dw_loc_list_table); } /* Output a marker (i.e. a label) for the beginning of the generated code for @@ -22210,6 +22283,11 @@ dwarf2out_init (const char *filename ATT decl_loc_table = htab_create_ggc (10, decl_loc_table_hash, decl_loc_table_eq, NULL); + /* Allocate the cached_dw_loc_list_table. */ + cached_dw_loc_list_table + = htab_create_ggc (10, cached_dw_loc_list_table_hash, + cached_dw_loc_list_table_eq, NULL); + /* Allocate the initial hunk of the decl_scope_table. */ decl_scope_table = VEC_alloc (tree, gc, 256); @@ -22852,30 +22930,53 @@ resolve_addr (dw_die_ref die) { dw_die_ref c; dw_attr_ref a; - dw_loc_list_ref *curr; + dw_loc_list_ref *curr, *start, loc; unsigned ix; FOR_EACH_VEC_ELT (dw_attr_node, die->die_attr, ix, a) switch (AT_class (a)) { case dw_val_class_loc_list: - curr = AT_loc_list_ptr (a); - while (*curr) + start = curr = AT_loc_list_ptr (a); + loc = *curr; + gcc_assert (loc); + /* The same list can be referenced more than once. See if we have + already recorded the result from a previous pass. */ + if (loc->replaced) + *curr = loc->dw_loc_next; + else if (!loc->resolved_addr) { - if (!resolve_addr_in_expr ((*curr)->expr)) + /* As things stand, we do not expect or allow one die to + reference a suffix of another die's location list chain. + References must be identical or completely separate. + There is therefore no need to cache the result of this + pass on any list other than the first; doing so + would lead to unnecessary writes. */ + while (*curr) { - dw_loc_list_ref next = (*curr)->dw_loc_next; - if (next && (*curr)->ll_symbol) + gcc_assert (!(*curr)->replaced && !(*curr)->resolved_addr); + if (!resolve_addr_in_expr ((*curr)->expr)) { - gcc_assert (!next->ll_symbol); - next->ll_symbol = (*curr)->ll_symbol; + dw_loc_list_ref next = (*curr)->dw_loc_next; + if (next && (*curr)->ll_symbol) + { + gcc_assert (!next->ll_symbol); + next->ll_symbol = (*curr)->ll_symbol; + } + *curr = next; } - *curr = next; + else + curr = &(*curr)->dw_loc_next; } + if (loc == *start) + loc->resolved_addr = 1; else - curr = &(*curr)->dw_loc_next; + { + loc->replaced = 1; + loc->dw_loc_next = *start; + } } - if (!AT_loc_list (a)) + if (!*start) { remove_AT (die, a->dw_attr); ix--; @@ -23304,6 +23405,7 @@ dwarf2out_finish (const char *filename) add_location_or_const_value_attribute ( VEC_index (deferred_locations, deferred_locations_list, i)->die, VEC_index (deferred_locations, deferred_locations_list, i)->variable, + false, DW_AT_location); }