From patchwork Mon Feb 14 20:00:29 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [PR,debug/47106] account used vars only once Date: Mon, 14 Feb 2011 10:00:29 -0000 From: Alexandre Oliva X-Patchwork-Id: 83154 Message-Id: To: Richard Guenther Cc: gcc-patches@gcc.gnu.org, Jan Hubicka On Feb 14, 2011, Richard Guenther wrote: > referenced-var-lookup-fn-arg.patch, for-each-referenced-var-fn-arg.patch, > estimated-stack-frame-size-cgraph-node-arg.patch and > compute-inline-params-ret-void.patch are obvious (and ok), you can apply > them independently now. Thanks > which does look like a hack. Looking at decl_address_invariant_p it > _seems_ that the test > || DECL_CONTEXT (op) == current_function_decl > || decl_function_context (op) == current_function_decl) > could simply be > DECL_CONTEXT (op) && TREE_CODE (DECL_CONTEX (op)) == FUNCTION_DECL Err... How would this tell whether the decl is local to the current function? > that said, I'm not sure about the necessity of this patch at this stage, > I'd like to defer it (or rather a better solution) to 4.7 if possible. > (you might want to open a bug so we don't forget) *nod* > Does dont-push-cfun-in-estimate-stack-frame-size.patch depend on this > patch? Nope. > I guess dont-push-cfun-in-estimate-stack-frame-size.patch depends on > ipa-inline-stack-growth-referenced-vars-pr47106.patch at least? Yup. > Similar dont-push-cfun-in-analyze-function.patch? Yep, but the analyze_function change also depends on the current_function_decl setting within the body size estimation. > ipa-inline-stack-growth-referenced-vars-pr47106.patch was attached twice, Oops, sorry, I meant to attach the -compare patch the second time. > so I'm not sure about inter-dependencies. If the do-not-set-cfun patches > pass testing ontop of the obvious cleanup patches they are ok as well. Ok, I'll retest and install. > Now onto ipa-inline-stack-growth-referenced-vars-pr47106.patch > it looks ok apart from the new pass_inlinable stuff, that at least sounds > odd and I'd want Honza to have a look here. The testcase you say > the patch breaks otherwise redefines a _static_ inline function Err, no, it's 3 separate functions with different names (foo, __foo and ____foo) > I also wonder why computing -> inlineable should have any effect on > that testcase ... Computing inlinable early enables edges to not be marked as non-inlinable early. Computing disregard_* early enables cycles to be broken at the right spots when deciding the order in which we're going to process functions. See the comment right before the two occurrences of disregard in ipa.c:cgraph_postorder(). > So, can we get the approved pre-requesties applied? Will do shortly. Thanks for the reviews. FTR, testing and reporting only, not to be installed. Index: gcc/cfgexpand.c =================================================================== --- gcc/cfgexpand.c.orig 2011-02-10 09:27:16.004373533 -0200 +++ gcc/cfgexpand.c 2011-02-10 09:29:49.418374663 -0200 @@ -1311,18 +1311,41 @@ create_stack_guard (void) crtl->stack_protect_guard = guard; } +/* A subroutine of expand_used_vars. Walk down through the BLOCK tree + expanding variables. Those variables that can be put into registers + are allocated pseudos; those that can't are put on the stack. + + TOPLEVEL is true if this is the outermost BLOCK. */ + +static HOST_WIDE_INT +account_used_vars_for_block (tree block, bool toplevel) +{ + tree t; + HOST_WIDE_INT size = 0; + + /* Expand all variables at this level. */ + for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t)) + size += expand_one_var (t, toplevel, false); + + /* Expand all variables at containing levels. */ + for (t = BLOCK_SUBBLOCKS (block); t ; t = BLOCK_CHAIN (t)) + size += account_used_vars_for_block (t, false); + + return size; +} + /* Prepare for expanding variables. */ static void -init_vars_expansion (void) +init_vars_expansion (tree decl) { tree t; unsigned ix; /* Set TREE_USED on all variables in the local_decls. */ - FOR_EACH_LOCAL_DECL (cfun, ix, t) + FOR_EACH_LOCAL_DECL (DECL_STRUCT_FUNCTION (decl), ix, t) TREE_USED (t) = 1; /* Clear TREE_USED on all variables associated with a block scope. */ - clear_tree_used (DECL_INITIAL (current_function_decl)); + clear_tree_used (DECL_INITIAL (decl)); /* Initialize local stack smashing state. */ has_protected_decls = false; @@ -1354,6 +1377,8 @@ HOST_WIDE_INT estimated_stack_frame_size (struct cgraph_node *node) { HOST_WIDE_INT size = 0; + HOST_WIDE_INT delta; + unsigned ix; size_t i; tree var; referenced_var_iterator rvi; @@ -1377,6 +1402,34 @@ estimated_stack_frame_size (struct cgrap fini_vars_expansion (); } + delta = size; + + init_vars_expansion (node->decl); + + FOR_EACH_LOCAL_DECL (fn, ix, var) + { + /* TREE_USED marks local variables that do not appear in lexical + blocks. We don't want to expand those that do twice. */ + if (TREE_USED (var)) + delta -= expand_one_var (var, true, false); + } + delta -= account_used_vars_for_block (DECL_INITIAL (node->decl), true); + + if (stack_vars_num > 0) + { + /* Fake sorting the stack vars for account_stack_vars (). */ + stack_vars_sorted = XNEWVEC (size_t, stack_vars_num); + for (i = 0; i < stack_vars_num; ++i) + stack_vars_sorted[i] = i; + delta -= account_stack_vars (); + fini_vars_expansion (); + } + + if (delta) + inform (input_location, + "guestimated stack size for %q+D changed by %li to %li", + node->decl, (long)delta, (long)size); + current_function_decl = save; pop_cfun (); @@ -1400,7 +1453,7 @@ expand_used_vars (void) frame_phase = off ? align - off : 0; } - init_vars_expansion (); + init_vars_expansion (current_function_decl); for (i = 0; i < SA.map->num_partitions; i++) {