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

[Jan Hubicka]

> 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);
> +    *dst_size = *src_size;
>      ipa_update_overall_fn_summary (dst);
> +  }

Thanks for spotting this! It is quite bad bug.
The summaries are supposed to be copied by duplicate method. However it
seems that the default duplicate implementation doesn't do the copy (I
wonder why) and moreover copy constructor is broken not copying
correctly stack use.  I think we are fine with the default copy
constructor as follows

Does it fix your testcase?

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