[PR debug/47106] account used vars only once

[Richard Guenther]

On Mon, Feb 7, 2011 at 11:30 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> 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.

Yes, we can have cycles and no referenced vars in the callee.  But
we won't inline that callee (as it isn't in SSA form), so we can just
as well skip looking at its estimated stack frame size.

>> 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.

I think you are now feeding more functions into re-optimization
(that's another reason I want to avoid the pass reorg for stage4).

Richard.

>
> --
> 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
>