[PATCH] [RFC] ipa: duplicate ipa_size_summary for cloned nodes

[luoxhu]

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  <luoxhu@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);
>   }
>