This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR debug/47106] account used vars only once
On Feb 7, 2011, Richard Guenther <richard.guenther@gmail.com> wrote:
> The referenced_var_lookup interface change is ok.
Ok, I'll split it out and check it in.
> tree old_cur_fun_decl = current_function_decl;
> + referenced_var_iterator rvi;
> +
> + gcc_checking_assert (gimple_referenced_vars (cfun));
> +
> current_function_decl = decl;
> push_cfun (DECL_STRUCT_FUNCTION (decl));
> looks like only the assert after pushing cfun is required?
Doh! Thanks.
> Can we get away w/o cfun pushing here now?
Maybe. I'll have a look.
> Index: gcc/tree-inline.c
> ===================================================================
> --- gcc/tree-inline.c.orig 2011-02-07 09:23:32.407058212 -0200
> +++ gcc/tree-inline.c 2011-02-07 09:25:20.376780499 -0200
> @@ -317,7 +317,11 @@ remap_decl (tree decl, copy_body_data *i
> || TREE_CODE (t) == RESULT_DECL || TREE_CODE (t) == PARM_DECL))
> {
> get_var_ann (t);
> - add_referenced_var (t);
> + if (TREE_CODE (decl) != VAR_DECL
> + || !gimple_referenced_vars (DECL_STRUCT_FUNCTION (id->src_fn))
> + || referenced_var_lookup (DECL_STRUCT_FUNCTION (id->src_fn),
> + DECL_UID (decl)))
> + add_referenced_var (t);
> get_var_ann is redundant with add_referenced_var.
Yeah, will drop and unminimize the patch.
> Do we really run into the
> !gimple_referenced_vars case here?
Some earlier version of the patch did. Maybe not any more.
> @@ -4753,6 +4757,13 @@ copy_decl_for_dup_finish (copy_body_data
> new function. */
> DECL_CONTEXT (copy) = id->dst_fn;
> + if (TREE_CODE (decl) == VAR_DECL
> + && gimple_referenced_vars (cfun)
> + && gimple_referenced_vars (DECL_STRUCT_FUNCTION (id->src_fn))
> + && referenced_var_lookup (DECL_STRUCT_FUNCTION (id->src_fn),
> + DECL_UID (decl)))
> + add_referenced_var (copy);
> Why only for VAR_DECL decls and not to-var-decl translated PARM_DECLs?
Yeah, PARM_DECLs and RESULT_DECLs get unconditional add_referenced_var
elsewhere.
> I wanted to ask you about
>> That isn't enough. We often attempt to estimate the stack size of a
>> function before we as much as put it in SSA form (let alone compute
>> referenced_vras), while processing other functions.
> anyway, as I don't see why we do that (nor where).
What I noticed was that the trivial patch that moved
pass_inline_parameters down and asserted gimple_referenced_vars(cfun)
failed the assertion. I didn't investigate much, I just figured it made
sense for one function to look at stack size estimates of its
potentially-inlined callees to estimate its own, and since the call
graph may contain cycles, we can't ensure we'll always process callee
before caller. If my âfiguringâ is right, I don't see how to fix this,
but I'll go back to the drawing board and investigate further.
> The cgraphunit.c change looks ok in this regard, can you explain
> the predict.c change?
We split a function after profile-guessing, and initialize_cfun copied
the PROFILE_GUESSED status from the original function to the split.
Then, when we got to running the various passes over the newly-created
version, we failed the assertion that profile_status != PROFILE_GUESSED
in gimple_predict_edge. I didn't try to find out why this didn't happen
before the pass rearrangement. I figured it made sense that the pass
rearrangement enabled some more inlining/versioning, exposing this kind
of situation just like it did expose the need for cleanups after early
inlining to avoid getting SSA verify failures because formerly-throwing
call stmts ended up known not to throw.
--
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 Brazil Compiler Engineer