Re: gcc-4_9 inlines less funcs than gcc-4_8 because of used_as_abstract_origin flag.

[Wei Mi <wmi at google dot com>]

Submit a bug here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63970

Wei.

On Wed, Nov 19, 2014 at 2:18 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Wed, Nov 19, 2014 at 8:21 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On November 19, 2014 8:13:09 AM CET, Wei Mi <wmi@google.com> wrote:
>>>We see an inline problem as below caused by r201408
>>>(https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00027.html).
>>>
>>>hoo() {
>>>  foo();
>>>  ...
>>>}
>>>
>>>foo {
>>>  goo();
>>>  ...
>>>}
>>>
>>>foo is func splitted, so its body changes to
>>>
>>>foo {
>>>  goo();
>>>  ...
>>>  foo.part();
>>>}
>>>
>>>and the used_as_abstract_origin of cgraph node of foo will be set to
>>>true after func splitting.
>>>
>>>In ipa-inline, when inlining foo into hoo, the original node of foo
>>>will not be reused as clone node because used_as_abstract_origin of
>>>cgraph node of foo is true and can_remove_node_now_p_1 will return
>>>false, so that a new clone node of foo will be created. This is the
>>>case in gcc-4_9.
>>>In gcc-4_8, the original node of foo will be reused as clone node.
>>>
>>>gcc-4_8
>>>foo
>>>  |
>>>goo
>>>
>>>gcc-4_9
>>>foo        foo_clone
>>>    \        /
>>>      goo
>>>
>>>Because of the difference of whether to create a new clone for foo,
>>>when inlining goo to foo, the overall growth of inlining all callsites
>>>of goo in gcc-4_8 will be less than gcc-4_9 (goo has two callsites in
>>>gcc-4_9 but only one in gcc-4_8). If we have many cases like this,
>>>gcc-4_8 will actually have more inline growth budget than gcc-4_9 and
>>>will inline more aggressively than gcc-4_9.
>>>
>>>I don't understand the exact usage of the check about
>>>node->used_as_abstract_origin in can_remove_node_now_p_1, but I feel
>>>puzzled about following two points:
>>>
>>>1. https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00027.html said the
>>>patch was to ensure all abstract origin functions do have nodes
>>>attached. However, even if the node of origin function is reused as a
>>>clone node, a new clone node will be created in following code in
>>>symbol_table::remove_unreachable_nodes if only the node that needs
>>>abstract origin is reachable.
>>>
>>>          if (TREE_CODE (node->decl) == FUNCTION_DECL
>>>              && DECL_ABSTRACT_ORIGIN (node->decl))
>>>            {
>>>              struct cgraph_node *origin_node
>>>         = cgraph_node::get_create (DECL_ABSTRACT_ORIGIN (node->decl));
>>>              origin_node->used_as_abstract_origin = true;
>>>              enqueue_node (origin_node, &first, &reachable);
>>>            }
>>>
>>>2. DECL_ABSTRACT_ORIGIN(decl) seems only useful for debug info of
>>>clone nodes. But now the check of used_as_abstract_origin affect
>>>inline decisions, which should be the same with or without keeping
>>>debug info.
>>
>> I think we need to keep the functions but do not need to account for them in the unit size if we otherwise could remove them
>
> Btw - please make sure to open a bug so this issue doesn't get lost.
>
> Thanks,
> Richard.
>
>> Richard.
>>
>>>Thanks,
>>>Wei.
>>
>>