From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17899 invoked by alias); 4 Mar 2015 15:38:25 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 17884 invoked by uid 89); 4 Mar 2015 15:38:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KAM_FROM_URIBL_PCCC,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no 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; Wed, 04 Mar 2015 15:38:15 +0000 Received: by obcwo20 with SMTP id wo20so7573157obc.7 for ; Wed, 04 Mar 2015 07:38:13 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.60.82.170 with SMTP id j10mr3471032oey.7.1425483493165; Wed, 04 Mar 2015 07:38:13 -0800 (PST) Received: by 10.76.98.137 with HTTP; Wed, 4 Mar 2015 07:38:13 -0800 (PST) In-Reply-To: References: Date: Wed, 04 Mar 2015 15:38:00 -0000 Message-ID: Subject: Re: [PR58315] reset inlined debug vars at return-to point From: Richard Biener To: Alexandre Oliva Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00226.txt.bz2 On Wed, Feb 25, 2015 at 10:40 AM, Alexandre Oliva wrote: > This patch fixes a problem that has been with us for several years. > Variable tracking has no idea about the end of the lifetime of inlined > variables, so it keeps on computing locations for them over and over, > even though the computed locations make no sense whatsoever because the > variable can't even be accessed any more. > > With this patch, we unbind all inlined variables at the point the > inlined function returns to, so that the locations for those variables > will not be touched any further. > > In theory, we could do something similar to non-inlined auto variables, > when they go out of scope, but their decls apply to the entire function > and I'm told gdb sort-of expects the variables to be accessible > throughout the function, so I'm not tackling that in this patch, for I'm > happy enough with what this patch gets us: > > - almost 99% reduction in the output asm for the PR testcase > > - more than 90% reduction in the peak memory use compiling that testcase > > - 63% reduction in the compile time for that testcase > > What's scary is that the testcase is not particularly pathological. Any > function that calls a longish sequence of inlined functions, that in > turn call other inline functions, and so on, something that's not > particularly unusual in C++, will likely observe significant > improvement, as we won't see growing sequences of var_location notes > after each call or so, as var-tracking computes a new in-stack location > for the implicit this argument of each previously-inlined function. > > Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? Some numbers for tramp3d at -Ofast -g with an optimized, release checking (but not bootstrapped) trunk. Compile-time memory usage seems unchanged (~980MB, to show differences we'd probably need to ggc-collect more often). Compile-time was slightly faster with the patch, 45s vs. 47s, but the machine wasn't completely un-loaded. var-tracking parts: unpatched: variable tracking : 0.63 ( 1%) usr 0.03 ( 1%) sys 0.82 ( 2%) wall 28641 kB ( 2%) ggc var-tracking dataflow : 3.72 ( 8%) usr 0.04 ( 1%) sys 3.65 ( 7%) wall 1337 kB ( 0%) ggc var-tracking emit : 2.63 ( 6%) usr 0.02 ( 1%) sys 2.55 ( 5%) wall 148835 kB ( 8%) ggc patched: variable tracking : 0.64 ( 1%) usr 0.01 ( 0%) sys 0.72 ( 1%) wall 24202 kB ( 1%) ggc var-tracking dataflow : 1.96 ( 4%) usr 0.01 ( 0%) sys 1.94 ( 4%) wall 1326 kB ( 0%) ggc var-tracking emit : 1.46 ( 3%) usr 0.02 ( 0%) sys 1.49 ( 3%) wall 46980 kB ( 3%) ggc we have at the point of RTL expansion 56% more debug statements (988231 lines with # DEBUG in the .optimized dump out of 1212518 lines in total vs. 630666 out of 854953). So we go from roughly 1 debug stmt per real stmt to 1.5 debug stmts per real stmt. I didn't try to inspect generated debug information or asses its quality. But I wonder for example what we do for _25 = MEM[(const struct DomainD.47403 *)this_11(D) + 8B].D.47690.domain_mD.47464; # DEBUG dD.570173 => _25 # DEBUG dD.570173 => NULL # DEBUG thisD.572552 => NULL npc_14 = _25 / _27; # DEBUG npcD.171771 => npc_14 # DEBUG D#447 => &this_11(D)->blocks_mD.64412.D.47809 # DEBUG thisD.572554 => D#447 # DEBUG dD.570173 => _25 # DEBUG dD.570173 => NULL # DEBUG thisD.572554 => NULL remainder_16 = _25 % _27; we have two pairs of DEBUG stmts for dD.570173 here, binding to _25 and then immediately resetting. Apart from that we might possibly DCE those I wonder if this isn't breaking debug experience vs. what we had previously: _25 = MEM[(const struct DomainD.47403 *)this_11(D) + 8B].D.47690.domain_mD.47464; # DEBUG dD.570173 => _25 npc_14 = _25 / _27; # DEBUG npcD.171771 => npc_14 # DEBUG D#447 => &this_11(D)->blocks_mD.64412.D.47809 # DEBUG thisD.572554 => D#447 # DEBUG dD.570173 => _25 remainder_16 = _25 % _27; dD.570173 is never reset anywhere in the function without the patch. Just for curiosity here is the last dump piece again, this time with location information (we should dump the associated BLOCK someow) [tramp3d-v4.cpp:3457:45] _25 = [tramp3d-v4.cpp:3457:45] MEM[(const struct DomainD.47403 *)this_11(D) + 8B].D.47690.domain_mD.47464; [tramp3d-v4.cpp:3457:45] # DEBUG dD.570173 => _25 [tramp3d-v4.cpp:53176:32] npc_14 = _25 / _27; [tramp3d-v4.cpp:53176:32] # DEBUG npcD.171771 => npc_14 [tramp3d-v4.cpp:53177:33] # DEBUG D#447 => [tramp3d-v4.cpp:53177:33] &[tramp3d-v4.cpp:53177:19] this_11(D)->blocks_mD.64412.D.47809 [tramp3d-v4.cpp:53177:33] # DEBUG thisD.572554 => D#447 [tramp3d-v4.cpp:3457:45] # DEBUG dD.570173 => _25 [tramp3d-v4.cpp:53177:38] remainder_16 = _25 % _27; like 3457 is inline Element_t first() const { return DT::first(this->domain_m); } and 5317[67] are int npc = blocks_m.first() / contexts; int remainder = blocks_m.first() % contexts; 'd' comes from DT::first(this->domain_m); which is on line 4604: inline static Element_t first(Storage_t d) { return d; } so it assigns a name to this->domain_m but we don't have any assembler instruction left for that BLOCK. Which means my pragmatic approach would have re-set the variable after the second bind only. We likely still keep the inline BLOCK (just wonder what PC range we assign to it and if I could break on it in gdb...). Richard. > Reset inlined debug variables at the end of the inlined function > > From: Alexandre Oliva > > for gcc/ChangeLog > > PR debug/58315 > * tree-inline.c (reset_debug_binding): New. > (reset_debug_bindings): Likewise. > (expand_call_inline): Call it. > --- > gcc/tree-inline.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c > index d8abe03..5b58d8b 100644 > --- a/gcc/tree-inline.c > +++ b/gcc/tree-inline.c > @@ -4345,6 +4345,60 @@ add_local_variables (struct function *callee, struct function *caller, > } > } > > +/* Add to BINDINGS a debug stmt resetting SRCVAR if inlining might > + have brought in or introduced any debug stmts for SRCVAR. */ > + > +static inline void > +reset_debug_binding (copy_body_data *id, tree srcvar, gimple_seq *bindings) > +{ > + tree *remappedvarp = id->decl_map->get (srcvar); > + > + if (!remappedvarp) > + return; > + > + if (TREE_CODE (*remappedvarp) != VAR_DECL) > + return; > + > + if (*remappedvarp == id->retvar || *remappedvarp == id->retbnd) > + return; > + > + tree tvar = target_for_debug_bind (*remappedvarp); > + if (!tvar) > + return; > + > + gdebug *stmt = gimple_build_debug_bind (tvar, NULL_TREE, > + id->call_stmt); > + gimple_seq_add_stmt (bindings, stmt); > +} > + > +/* For each inlined variable for which we may have debug bind stmts, > + add before GSI a final debug stmt resetting it, marking the end of > + its life, so that var-tracking knows it doesn't have to compute > + further locations for it. */ > + > +static inline void > +reset_debug_bindings (copy_body_data *id, gimple_stmt_iterator gsi) > +{ > + tree var; > + unsigned ix; > + gimple_seq bindings = NULL; > + > + if (!gimple_in_ssa_p (id->src_cfun)) > + return; > + > + if (!opt_for_fn (id->dst_fn, flag_var_tracking_assignments)) > + return; > + > + for (var = DECL_ARGUMENTS (id->src_fn); > + var; var = DECL_CHAIN (var)) > + reset_debug_binding (id, var, &bindings); > + > + FOR_EACH_LOCAL_DECL (id->src_cfun, ix, var) > + reset_debug_binding (id, var, &bindings); > + > + gsi_insert_seq_before_without_update (&gsi, bindings, GSI_SAME_STMT); > +} > + > /* If STMT is a GIMPLE_CALL, replace it with its inline expansion. */ > > static bool > @@ -4650,6 +4704,8 @@ expand_call_inline (basic_block bb, gimple stmt, copy_body_data *id) > GCOV_COMPUTE_SCALE (cg_edge->frequency, CGRAPH_FREQ_BASE), > bb, return_block, NULL); > > + reset_debug_bindings (id, stmt_gsi); > + > /* Reset the escaped solution. */ > if (cfun->gimple_df) > pt_solution_reset (&cfun->gimple_df->escaped); > > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer