Patchwork [GOOGLE] Restrict the count_scale to be no larger than 100%

login
register
mail settings
Submitter Dehao Chen
Date Jan. 17, 2014, 10:38 p.m.
Message ID <CAO2gOZXsLsQhyd2Kiw9hXhzKqUpVqjPvfThY=JAmrwci2uW3Sg@mail.gmail.com>
Download mbox | patch
Permalink /patch/312211/
State New
Headers show

Comments

Dehao Chen - Jan. 17, 2014, 10:38 p.m.
In AutoFDO, sometime edge count might be propagated to be too large
due to bad debug info. In this cases, we need to make sure the count
scale is no larger than 100% otherwise it'll make real hot code cold.

Bootstrapped and passed regression test. Performance test on-going.

OK for google-4_8 if performance test is ok?

Thanks,
Dehao
Xinliang David Li - Jan. 17, 2014, 11:12 p.m.
Can callgraph node count be fixed up properly instead of doing
individual fixups like this?

David

On Fri, Jan 17, 2014 at 2:38 PM, Dehao Chen <dehao@google.com> wrote:
> In AutoFDO, sometime edge count might be propagated to be too large
> due to bad debug info. In this cases, we need to make sure the count
> scale is no larger than 100% otherwise it'll make real hot code cold.
>
> Bootstrapped and passed regression test. Performance test on-going.
>
> OK for google-4_8 if performance test is ok?
>
> Thanks,
> Dehao
>
> Index: gcc/tree-inline.c
> ===================================================================
> --- gcc/tree-inline.c (revision 206721)
> +++ gcc/tree-inline.c (working copy)
> @@ -2262,6 +2262,9 @@ copy_cfg_body (copy_body_data * id, gcov_type coun
>    else
>      count_scale = REG_BR_PROB_BASE;
>
> +  if (flag_auto_profile && count_scale > REG_BR_PROB_BASE)
> +    count_scale = REG_BR_PROB_BASE;
> +
>    /* Register specific tree functions.  */
>    gimple_register_cfg_hooks ();
>
> Index: gcc/cgraphclones.c
> ===================================================================
> --- gcc/cgraphclones.c (revision 206721)
> +++ gcc/cgraphclones.c (working copy)
> @@ -216,7 +216,10 @@ cgraph_clone_node (struct cgraph_node *n, tree dec
>       count, we will not update the original callee because it may
>       mistakenly mark some hot function as cold.  */
>    if (flag_auto_profile && count >= n->count)
> -    update_original = false;
> +    {
> +      update_original = false;
> +      new_node->count = n->count;
> +    }
>    if (update_original)
>      {
>        n->count -= count;
Dehao Chen - Jan. 22, 2014, 11:23 p.m.
Unfortunately, copy_cfg_body is actually using basic block count
instead of cgraph edge count. Thus even fixing up the call graph does
not solve the problem. The 2nd chunk of the patch (cgraphclones.c) is
actually not necessary. We only need the first part (tree-inline.c).

Thanks,
Dehao

On Fri, Jan 17, 2014 at 3:12 PM, Xinliang David Li <davidxl@google.com> wrote:
> Can callgraph node count be fixed up properly instead of doing
> individual fixups like this?
>
> David
>
> On Fri, Jan 17, 2014 at 2:38 PM, Dehao Chen <dehao@google.com> wrote:
>> In AutoFDO, sometime edge count might be propagated to be too large
>> due to bad debug info. In this cases, we need to make sure the count
>> scale is no larger than 100% otherwise it'll make real hot code cold.
>>
>> Bootstrapped and passed regression test. Performance test on-going.
>>
>> OK for google-4_8 if performance test is ok?
>>
>> Thanks,
>> Dehao
>>
>> Index: gcc/tree-inline.c
>> ===================================================================
>> --- gcc/tree-inline.c (revision 206721)
>> +++ gcc/tree-inline.c (working copy)
>> @@ -2262,6 +2262,9 @@ copy_cfg_body (copy_body_data * id, gcov_type coun
>>    else
>>      count_scale = REG_BR_PROB_BASE;
>>
>> +  if (flag_auto_profile && count_scale > REG_BR_PROB_BASE)
>> +    count_scale = REG_BR_PROB_BASE;
>> +
>>    /* Register specific tree functions.  */
>>    gimple_register_cfg_hooks ();
>>
>> Index: gcc/cgraphclones.c
>> ===================================================================
>> --- gcc/cgraphclones.c (revision 206721)
>> +++ gcc/cgraphclones.c (working copy)
>> @@ -216,7 +216,10 @@ cgraph_clone_node (struct cgraph_node *n, tree dec
>>       count, we will not update the original callee because it may
>>       mistakenly mark some hot function as cold.  */
>>    if (flag_auto_profile && count >= n->count)
>> -    update_original = false;
>> +    {
>> +      update_original = false;
>> +      new_node->count = n->count;
>> +    }
>>    if (update_original)
>>      {
>>        n->count -= count;
Xinliang David Li - Jan. 22, 2014, 11:48 p.m.
ok.

David

On Wed, Jan 22, 2014 at 3:23 PM, Dehao Chen <dehao@google.com> wrote:
> Unfortunately, copy_cfg_body is actually using basic block count
> instead of cgraph edge count. Thus even fixing up the call graph does
> not solve the problem. The 2nd chunk of the patch (cgraphclones.c) is
> actually not necessary. We only need the first part (tree-inline.c).
>
> Thanks,
> Dehao
>
> On Fri, Jan 17, 2014 at 3:12 PM, Xinliang David Li <davidxl@google.com> wrote:
>> Can callgraph node count be fixed up properly instead of doing
>> individual fixups like this?
>>
>> David
>>
>> On Fri, Jan 17, 2014 at 2:38 PM, Dehao Chen <dehao@google.com> wrote:
>>> In AutoFDO, sometime edge count might be propagated to be too large
>>> due to bad debug info. In this cases, we need to make sure the count
>>> scale is no larger than 100% otherwise it'll make real hot code cold.
>>>
>>> Bootstrapped and passed regression test. Performance test on-going.
>>>
>>> OK for google-4_8 if performance test is ok?
>>>
>>> Thanks,
>>> Dehao
>>>
>>> Index: gcc/tree-inline.c
>>> ===================================================================
>>> --- gcc/tree-inline.c (revision 206721)
>>> +++ gcc/tree-inline.c (working copy)
>>> @@ -2262,6 +2262,9 @@ copy_cfg_body (copy_body_data * id, gcov_type coun
>>>    else
>>>      count_scale = REG_BR_PROB_BASE;
>>>
>>> +  if (flag_auto_profile && count_scale > REG_BR_PROB_BASE)
>>> +    count_scale = REG_BR_PROB_BASE;
>>> +
>>>    /* Register specific tree functions.  */
>>>    gimple_register_cfg_hooks ();
>>>
>>> Index: gcc/cgraphclones.c
>>> ===================================================================
>>> --- gcc/cgraphclones.c (revision 206721)
>>> +++ gcc/cgraphclones.c (working copy)
>>> @@ -216,7 +216,10 @@ cgraph_clone_node (struct cgraph_node *n, tree dec
>>>       count, we will not update the original callee because it may
>>>       mistakenly mark some hot function as cold.  */
>>>    if (flag_auto_profile && count >= n->count)
>>> -    update_original = false;
>>> +    {
>>> +      update_original = false;
>>> +      new_node->count = n->count;
>>> +    }
>>>    if (update_original)
>>>      {
>>>        n->count -= count;

Patch

Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c (revision 206721)
+++ gcc/tree-inline.c (working copy)
@@ -2262,6 +2262,9 @@  copy_cfg_body (copy_body_data * id, gcov_type coun
   else
     count_scale = REG_BR_PROB_BASE;

+  if (flag_auto_profile && count_scale > REG_BR_PROB_BASE)
+    count_scale = REG_BR_PROB_BASE;
+
   /* Register specific tree functions.  */
   gimple_register_cfg_hooks ();

Index: gcc/cgraphclones.c
===================================================================
--- gcc/cgraphclones.c (revision 206721)
+++ gcc/cgraphclones.c (working copy)
@@ -216,7 +216,10 @@  cgraph_clone_node (struct cgraph_node *n, tree dec
      count, we will not update the original callee because it may
      mistakenly mark some hot function as cold.  */
   if (flag_auto_profile && count >= n->count)
-    update_original = false;
+    {
+      update_original = false;
+      new_node->count = n->count;
+    }
   if (update_original)
     {
       n->count -= count;