This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Updates ssa and inline summary in the correct location for AutoFDO
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Jan Hubicka <hubicka at ucw dot cz>
- Cc: Dehao Chen <dehao at google dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, David Li <davidxl at google dot com>
- Date: Wed, 17 Dec 2014 10:42:06 +0100
- Subject: Re: [PATCH] Updates ssa and inline summary in the correct location for AutoFDO
- Authentication-results: sourceware.org; auth=none
- References: <CAO2gOZVSUP3jff+2TsZG8_ZNokS9gKZ9ZZZ3JydYOWUt-2+=Aw at mail dot gmail dot com> <CAO2gOZU4zNM1HGRvKP+pn5aMfC=a=JChyD1iX19oj+kUN5mU8w at mail dot gmail dot com> <20141216225311 dot GD4831 at kam dot mff dot cuni dot cz>
On Tue, Dec 16, 2014 at 11:53 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > gcc/ChangeLog:
>> > 2014-11-18 Dehao Chen <dehao@google.com>
>> >
>> > * auto-profile.c (afdo_annotate_cfg): Invoke update_ssa in the right
>> > place.
>> > (auto_profile): Recompute inline summary after processing cgraph node.
>> >
>> > Index: gcc/auto-profile.c
>> > ===================================================================
>> > --- gcc/auto-profile.c (revision 217741)
>> > +++ gcc/auto-profile.c (working copy)
>> > @@ -1514,7 +1514,14 @@ afdo_annotate_cfg (const stmt_set &promoted_stmts)
>> > profile_status_for_fn (cfun) = PROFILE_READ;
>> > }
>> > if (flag_value_profile_transformations)
>> > - gimple_value_profile_transformations ();
>> > + {
>> > + gimple_value_profile_transformations ();
>> > + free_dominance_info (CDI_DOMINATORS);
>> > + free_dominance_info (CDI_POST_DOMINATORS);
>> > + calculate_dominance_info (CDI_POST_DOMINATORS);
>> > + calculate_dominance_info (CDI_DOMINATORS);
>> > + update_ssa (TODO_update_ssa);
>
> I do not think you need to calcualte post dominators
correct, freeing them is enough.
> and most likely
> update_ssa will do its own dominators clauclation if ndeeded.
No, it expects them to be computed and up-to-date.
> It would be prettier to get those done via TODOs and subpasses, but if it is impossible
> patch is OK with the change above.
Calling update_ssa manually is done quite often so I think it's ok.
Richard.
> What about the performance tweaks we separated out form the original auto-FDO commit?
>
> Honza
>> > + }
>> > }
>> >
>> > /* Wrapper function to invoke early inliner. */
>> > @@ -1592,7 +1599,6 @@ auto_profile (void)
>> > early_inline ();
>> > autofdo::afdo_annotate_cfg (promoted_stmts);
>> > compute_function_frequency ();
>> > - update_ssa (TODO_update_ssa);
>> >
>> > /* Local pure-const may imply need to fixup the cfg. */
>> > if (execute_fixup_cfg () & TODO_cleanup_cfg)
>> > @@ -1601,6 +1607,7 @@ auto_profile (void)
>> > free_dominance_info (CDI_DOMINATORS);
>> > free_dominance_info (CDI_POST_DOMINATORS);
>> > cgraph_edge::rebuild_edges ();
>> > + compute_inline_parameters (cgraph_node::get (current_function_decl), true);
>> > pop_cfun ();
>> > }