diff mbox

[GOOGLE] update ssa before compute_inline_parameters

Message ID CAO2gOZXPfai3P8DDjZ4Z0iyTqBgv-2o=U_9pyfRCHXHK-1u-aQ@mail.gmail.com
State New
Headers show

Commit Message

Dehao Chen March 20, 2014, 7:40 p.m. UTC 
Patch updated to add a wrapper early_inline function

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

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

Comments

Xinliang David Li March 20, 2014, 8:02 p.m. UTC | #1 
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);
>>>
Dehao Chen March 20, 2014, 8:10 p.m. UTC | #2 
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);
>>>>
Xinliang David Li March 20, 2014, 8:17 p.m. UTC | #3 
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);
>>>>>
diff mbox

Patch

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