Message ID | CAO2gOZXPfai3P8DDjZ4Z0iyTqBgv-2o=U_9pyfRCHXHK-1u-aQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Thu, Mar 20, 2014 at 12:40 PM, Dehao Chen <dehao@google.com> wrote: > Patch updated to add a wrapper early_inline function > > Index: gcc/auto-profile.c > =================================================================== > --- gcc/auto-profile.c (revision 208726) > +++ gcc/auto-profile.c (working copy) > @@ -1449,8 +1449,6 @@ afdo_vpt_for_early_inline (stmt_set *promoted_stmt > calculate_dominance_info (CDI_POST_DOMINATORS); > calculate_dominance_info (CDI_DOMINATORS); > rebuild_cgraph_edges (); > - update_ssa (TODO_update_ssa); > - compute_inline_parameters (cgraph_get_node > (current_function_decl), true); > return true; > } > else > @@ -1516,6 +1514,14 @@ afdo_annotate_cfg (const stmt_set &promoted_stmts) > } > } /* namespace autofdo. */ > > +static void early_inline () > +{ > + compute_inline_parameters (cgraph_get_node (current_function_decl), true); > + unsigned todo = early_inliner (); > + if (todo & TODO_update_ssa_any) > + update_ssa (TODO_update_ssa); > +} > + > /* Use AutoFDO profile to annoate the control flow graph. > Return the todo flag. */ > > @@ -1610,11 +1616,10 @@ auto_profile (void) > if (!flag_value_profile_transformations > || !autofdo::afdo_vpt_for_early_inline (&promoted_stmts)) > break; > - early_inliner (); > + early_inline (); > } > > - compute_inline_parameters (cgraph_get_node > (current_function_decl), true); > - early_inliner (); > + early_inline (); > autofdo::afdo_annotate_cfg (promoted_stmts); > compute_function_frequency (); > update_ssa (TODO_update_ssa); Is this update still needed? David > > On Thu, Mar 20, 2014 at 11:36 AM, Xinliang David Li <davidxl@google.com> wrote: >> I think the right way to fix this is to wrap the call to early_inliner >> and check the TODO flags. See execute_function_todo: >> >> if (flags & TODO_cleanup_cfg) >> { >> cleanup_tree_cfg (); >> >> if (!(flags & TODO_update_ssa_any) && need_ssa_update_p (cfun)) >> flags |= TODO_update_ssa; >> } >> >> if (flags & TODO_update_ssa_any) >> { >> unsigned update_flags = flags & TODO_update_ssa_any; >> update_ssa (update_flags); >> cfun->last_verified &= ~TODO_verify_ssa; >> } >> >> David >> >> On Thu, Mar 20, 2014 at 10:39 AM, Dehao Chen <dehao@google.com> wrote: >>> This patch calls update_ssa before compute_inline_paramters. >>> >>> Bootstrapped and perf test on-going. >>> >>> OK for google-4_8? >>> >>> Thanks, >>> Dehao >>> >>> Index: gcc/auto-profile.c >>> =================================================================== >>> --- gcc/auto-profile.c (revision 208726) >>> +++ gcc/auto-profile.c (working copy) >>> @@ -1613,6 +1613,7 @@ auto_profile (void) >>> early_inliner (); >>> } >>> >>> + update_ssa (TODO_update_ssa); >>> compute_inline_parameters (cgraph_get_node (current_function_decl), >>> true); >>> early_inliner (); >>> autofdo::afdo_annotate_cfg (promoted_stmts); >>>
On Thu, Mar 20, 2014 at 1:02 PM, Xinliang David Li <davidxl@google.com> wrote: > On Thu, Mar 20, 2014 at 12:40 PM, Dehao Chen <dehao@google.com> wrote: >> Patch updated to add a wrapper early_inline function >> >> Index: gcc/auto-profile.c >> =================================================================== >> --- gcc/auto-profile.c (revision 208726) >> +++ gcc/auto-profile.c (working copy) >> @@ -1449,8 +1449,6 @@ afdo_vpt_for_early_inline (stmt_set *promoted_stmt >> calculate_dominance_info (CDI_POST_DOMINATORS); >> calculate_dominance_info (CDI_DOMINATORS); >> rebuild_cgraph_edges (); >> - update_ssa (TODO_update_ssa); >> - compute_inline_parameters (cgraph_get_node >> (current_function_decl), true); >> return true; >> } >> else >> @@ -1516,6 +1514,14 @@ afdo_annotate_cfg (const stmt_set &promoted_stmts) >> } >> } /* namespace autofdo. */ >> >> +static void early_inline () >> +{ >> + compute_inline_parameters (cgraph_get_node (current_function_decl), true); >> + unsigned todo = early_inliner (); >> + if (todo & TODO_update_ssa_any) >> + update_ssa (TODO_update_ssa); >> +} >> + >> /* Use AutoFDO profile to annoate the control flow graph. >> Return the todo flag. */ >> >> @@ -1610,11 +1616,10 @@ auto_profile (void) >> if (!flag_value_profile_transformations >> || !autofdo::afdo_vpt_for_early_inline (&promoted_stmts)) >> break; >> - early_inliner (); >> + early_inline (); >> } >> >> - compute_inline_parameters (cgraph_get_node >> (current_function_decl), true); >> - early_inliner (); >> + early_inline (); >> autofdo::afdo_annotate_cfg (promoted_stmts); >> compute_function_frequency (); >> update_ssa (TODO_update_ssa); > > Is this update still needed? Yes because in autofdo::afdo_annotate_cfg, gimple_value_profile_transformations is invoked. Dehao > > David > >> >> On Thu, Mar 20, 2014 at 11:36 AM, Xinliang David Li <davidxl@google.com> wrote: >>> I think the right way to fix this is to wrap the call to early_inliner >>> and check the TODO flags. See execute_function_todo: >>> >>> if (flags & TODO_cleanup_cfg) >>> { >>> cleanup_tree_cfg (); >>> >>> if (!(flags & TODO_update_ssa_any) && need_ssa_update_p (cfun)) >>> flags |= TODO_update_ssa; >>> } >>> >>> if (flags & TODO_update_ssa_any) >>> { >>> unsigned update_flags = flags & TODO_update_ssa_any; >>> update_ssa (update_flags); >>> cfun->last_verified &= ~TODO_verify_ssa; >>> } >>> >>> David >>> >>> On Thu, Mar 20, 2014 at 10:39 AM, Dehao Chen <dehao@google.com> wrote: >>>> This patch calls update_ssa before compute_inline_paramters. >>>> >>>> Bootstrapped and perf test on-going. >>>> >>>> OK for google-4_8? >>>> >>>> Thanks, >>>> Dehao >>>> >>>> Index: gcc/auto-profile.c >>>> =================================================================== >>>> --- gcc/auto-profile.c (revision 208726) >>>> +++ gcc/auto-profile.c (working copy) >>>> @@ -1613,6 +1613,7 @@ auto_profile (void) >>>> early_inliner (); >>>> } >>>> >>>> + update_ssa (TODO_update_ssa); >>>> compute_inline_parameters (cgraph_get_node (current_function_decl), >>>> true); >>>> early_inliner (); >>>> autofdo::afdo_annotate_cfg (promoted_stmts); >>>>
The patch is ok. David On Thu, Mar 20, 2014 at 1:10 PM, Dehao Chen <dehao@google.com> wrote: > On Thu, Mar 20, 2014 at 1:02 PM, Xinliang David Li <davidxl@google.com> wrote: >> On Thu, Mar 20, 2014 at 12:40 PM, Dehao Chen <dehao@google.com> wrote: >>> Patch updated to add a wrapper early_inline function >>> >>> Index: gcc/auto-profile.c >>> =================================================================== >>> --- gcc/auto-profile.c (revision 208726) >>> +++ gcc/auto-profile.c (working copy) >>> @@ -1449,8 +1449,6 @@ afdo_vpt_for_early_inline (stmt_set *promoted_stmt >>> calculate_dominance_info (CDI_POST_DOMINATORS); >>> calculate_dominance_info (CDI_DOMINATORS); >>> rebuild_cgraph_edges (); >>> - update_ssa (TODO_update_ssa); >>> - compute_inline_parameters (cgraph_get_node >>> (current_function_decl), true); >>> return true; >>> } >>> else >>> @@ -1516,6 +1514,14 @@ afdo_annotate_cfg (const stmt_set &promoted_stmts) >>> } >>> } /* namespace autofdo. */ >>> >>> +static void early_inline () >>> +{ >>> + compute_inline_parameters (cgraph_get_node (current_function_decl), true); >>> + unsigned todo = early_inliner (); >>> + if (todo & TODO_update_ssa_any) >>> + update_ssa (TODO_update_ssa); >>> +} >>> + >>> /* Use AutoFDO profile to annoate the control flow graph. >>> Return the todo flag. */ >>> >>> @@ -1610,11 +1616,10 @@ auto_profile (void) >>> if (!flag_value_profile_transformations >>> || !autofdo::afdo_vpt_for_early_inline (&promoted_stmts)) >>> break; >>> - early_inliner (); >>> + early_inline (); >>> } >>> >>> - compute_inline_parameters (cgraph_get_node >>> (current_function_decl), true); >>> - early_inliner (); >>> + early_inline (); >>> autofdo::afdo_annotate_cfg (promoted_stmts); >>> compute_function_frequency (); >>> update_ssa (TODO_update_ssa); >> >> Is this update still needed? > > Yes because in autofdo::afdo_annotate_cfg, > gimple_value_profile_transformations is invoked. > > Dehao > >> >> David >> >>> >>> On Thu, Mar 20, 2014 at 11:36 AM, Xinliang David Li <davidxl@google.com> wrote: >>>> I think the right way to fix this is to wrap the call to early_inliner >>>> and check the TODO flags. See execute_function_todo: >>>> >>>> if (flags & TODO_cleanup_cfg) >>>> { >>>> cleanup_tree_cfg (); >>>> >>>> if (!(flags & TODO_update_ssa_any) && need_ssa_update_p (cfun)) >>>> flags |= TODO_update_ssa; >>>> } >>>> >>>> if (flags & TODO_update_ssa_any) >>>> { >>>> unsigned update_flags = flags & TODO_update_ssa_any; >>>> update_ssa (update_flags); >>>> cfun->last_verified &= ~TODO_verify_ssa; >>>> } >>>> >>>> David >>>> >>>> On Thu, Mar 20, 2014 at 10:39 AM, Dehao Chen <dehao@google.com> wrote: >>>>> This patch calls update_ssa before compute_inline_paramters. >>>>> >>>>> Bootstrapped and perf test on-going. >>>>> >>>>> OK for google-4_8? >>>>> >>>>> Thanks, >>>>> Dehao >>>>> >>>>> Index: gcc/auto-profile.c >>>>> =================================================================== >>>>> --- gcc/auto-profile.c (revision 208726) >>>>> +++ gcc/auto-profile.c (working copy) >>>>> @@ -1613,6 +1613,7 @@ auto_profile (void) >>>>> early_inliner (); >>>>> } >>>>> >>>>> + update_ssa (TODO_update_ssa); >>>>> compute_inline_parameters (cgraph_get_node (current_function_decl), >>>>> true); >>>>> early_inliner (); >>>>> autofdo::afdo_annotate_cfg (promoted_stmts); >>>>>
Index: gcc/auto-profile.c =================================================================== --- gcc/auto-profile.c (revision 208726) +++ gcc/auto-profile.c (working copy) @@ -1449,8 +1449,6 @@ afdo_vpt_for_early_inline (stmt_set *promoted_stmt calculate_dominance_info (CDI_POST_DOMINATORS); calculate_dominance_info (CDI_DOMINATORS); rebuild_cgraph_edges (); - update_ssa (TODO_update_ssa); - compute_inline_parameters (cgraph_get_node (current_function_decl), true); return true; }