| Message ID | CAO2gOZVSUP3jff+2TsZG8_ZNokS9gKZ9ZZZ3JydYOWUt-2+=Aw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
ping... Thanks, Dehao On Tue, Nov 18, 2014 at 2:29 PM, Dehao Chen <dehao@google.com> wrote: > This patch updates ssa and inline summary in the correct location for AutoFDO. > > Bootstrapped and passed regression test. OK for trunk? > > Thanks, > Dehao > > 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); > + } > } > > /* 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 (); > }
> > 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 and most likely update_ssa will do its own dominators clauclation if ndeeded. It would be prettier to get those done via TODOs and subpasses, but if it is impossible patch is OK with the change above. 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 (); > > }
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 (); >> > }
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); + } } /* 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 (); }
This patch updates ssa and inline summary in the correct location for AutoFDO. Bootstrapped and passed regression test. OK for trunk? Thanks, Dehao 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.