On 2019/12/18 23:48, Jan Hubicka wrote:
>> The size_info of ipa_size_summary are created by r277424. It should be
>> duplicated for cloned nodes, otherwise self_size and
>> estimated_self_stack_size
>> would be 0, causing param large-function-insns and large-function-growth
>> working
>> inaccurate when ipa-inline.
>>
>> gcc/ChangeLog:
>>
>> 2019-12-18 Luo Xiong Hu <luo...@linux.ibm.com>
>>
>> * ipa-fnsummary.c (ipa_fn_summary_t::duplicate): Copy
>> ipa_size_summary for cloned nodes.
>> ---
>> gcc/ipa-fnsummary.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
>> index a46b1445765..9a01be1708b 100644
>> --- a/gcc/ipa-fnsummary.c
>> +++ b/gcc/ipa-fnsummary.c
>> @@ -868,7 +868,12 @@ ipa_fn_summary_t::duplicate (cgraph_node *src,
>> }
>> }
>> if (!dst->inlined_to)
>> + {
>> + class ipa_size_summary *src_size = ipa_size_summaries->get_create (src);
>> + class ipa_size_summary *dst_size = ipa_size_summaries->get_create (dst);
>
> This is intended to happen by the default duplicate method of
> ipa_size_summaries via to copy constructor. It seems there is a stupid
> pasto and the copy constructor is unused since the default duplicate
> implementation does nothing (wonder why).
>
> I am testing the attached patch. Does this help?
Yes, It works. Thanks for your refine. The default duplicate implementation
is in
symbol-summary.h:template <class T>class function_summary_base::duplicate, I
tried
to call duplicate in it, but it will cause a lot of errors as many classes
doesn't
implement the virtual duplicate function. Please commit your patch once tested
pass :)
Xiong Hu
>
> Index: ipa-fnsummary.h
> ===================================================================
> --- ipa-fnsummary.h (revision 279523)
> +++ ipa-fnsummary.h (working copy)
> @@ -99,11 +99,6 @@ public:
> : estimated_self_stack_size (0), self_size (0), size (0)
> {
> }
> - /* Copy constructor. */
> - ipa_size_summary (const ipa_size_summary &s)
> - : estimated_self_stack_size (0), self_size (s.self_size), size (s.size)
> - {
> - }
> };
>
> /* Function inlining information. */
> @@ -226,18 +221,20 @@ extern GTY(()) fast_function_summary <ip
> *ipa_fn_summaries;
>
> class ipa_size_summary_t:
> - public fast_function_summary <ipa_size_summary *, va_gc>
> + public fast_function_summary <ipa_size_summary *, va_heap>
> {
> public:
> ipa_size_summary_t (symbol_table *symtab):
> - fast_function_summary <ipa_size_summary *, va_gc> (symtab) {}
> + fast_function_summary <ipa_size_summary *, va_heap> (symtab)
> + {
> + disable_insertion_hook ();
> + }
>
> - static ipa_size_summary_t *create_ggc (symbol_table *symtab)
> + virtual void duplicate (cgraph_node *, cgraph_node *,
> + ipa_size_summary *src_data,
> + ipa_size_summary *dst_data)
> {
> - class ipa_size_summary_t *summary = new (ggc_alloc <ipa_size_summary_t>
> ())
> - ipa_size_summary_t (symtab);
> - summary->disable_insertion_hook ();
> - return summary;
> + *dst_data = *src_data;
> }
> };
> extern fast_function_summary <ipa_size_summary *, va_heap>
> Index: ipa-fnsummary.c
> ===================================================================
> --- ipa-fnsummary.c (revision 279523)
> +++ ipa-fnsummary.c (working copy)
> @@ -672,8 +672,7 @@ static void
> ipa_fn_summary_alloc (void)
> {
> gcc_checking_assert (!ipa_fn_summaries);
> - ipa_size_summaries = new fast_function_summary <ipa_size_summary *,
> va_heap>
> - (symtab);
> + ipa_size_summaries = new ipa_size_summary_t (symtab);
> ipa_fn_summaries = ipa_fn_summary_t::create_ggc (symtab);
> ipa_call_summaries = new ipa_call_summary_t (symtab);
> }
>
