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

[Richard Biener <richard dot guenther at gmail dot com>]

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

Richard.

>Thanks,
>Wei.