[PATCH, PR 60640] When creating virtual clones, clone thunks too

[PATCH, PR 60640] When creating virtual clones, clone thunks too

[Martin Jambor]

Hi,

this patch fixes PR 60640 by creating thunks to clones when that is
necessary to properly redirect edges to them.  I mostly does what
cgraph_add_thunk does and what analyze_function does to thunks.  It
fixes the testcases on trunk (it does not apply to 4.8, I have not
looked how easily fixable that it) and passes bootstrap and testing on
x86_64-linux.

OK for trunk?

Thanks,

Martin


2014-03-26  Martin Jambor  <mjambor@suse.cz>

	* cgraph.h (cgraph_clone_node): New parameter added to declaration.
	Adjust all callers.
	* cgraphclones.c (build_function_type_skip_args): Moved upwards in the
	file.
	(build_function_decl_skip_args): Likewise.
	(duplicate_thunk_for_node): New function.
	(redirect_edge_duplicating_thunks): Likewise.
	(cgraph_clone_node): New parameter args_to_skip, pass it to
	redirect_edge_duplicating_thunks which is called instead of
	cgraph_redirect_edge_callee.
	(cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node.

testsuite/
	* g++.dg/ipa/pr60640-1.C: New test.
	* g++.dg/ipa/pr60640-2.C: Likewise.

Index: src/gcc/cgraph.h
===================================================================
--- src.orig/gcc/cgraph.h
+++ src/gcc/cgraph.h
@@ -890,7 +890,7 @@ struct cgraph_edge * cgraph_clone_edge (
 					unsigned, gcov_type, int, bool);
 struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type,
 					int, bool, vec<cgraph_edge_p>,
-					bool, struct cgraph_node *);
+					bool, struct cgraph_node *, bitmap);
 tree clone_function_name (tree decl, const char *);
 struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node,
 			                          vec<cgraph_edge_p>,
Index: src/gcc/cgraphclones.c
===================================================================
--- src.orig/gcc/cgraphclones.c
+++ src/gcc/cgraphclones.c
@@ -168,6 +168,183 @@ cgraph_clone_edge (struct cgraph_edge *e
   return new_edge;
 }
 
+/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
+   return value if SKIP_RETURN is true.  */
+
+static tree
+build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
+			       bool skip_return)
+{
+  tree new_type = NULL;
+  tree args, new_args = NULL, t;
+  tree new_reversed;
+  int i = 0;
+
+  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
+       args = TREE_CHAIN (args), i++)
+    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
+      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
+
+  new_reversed = nreverse (new_args);
+  if (args)
+    {
+      if (new_reversed)
+        TREE_CHAIN (new_args) = void_list_node;
+      else
+	new_reversed = void_list_node;
+    }
+
+  /* Use copy_node to preserve as much as possible from original type
+     (debug info, attribute lists etc.)
+     Exception is METHOD_TYPEs must have THIS argument.
+     When we are asked to remove it, we need to build new FUNCTION_TYPE
+     instead.  */
+  if (TREE_CODE (orig_type) != METHOD_TYPE
+      || !args_to_skip
+      || !bitmap_bit_p (args_to_skip, 0))
+    {
+      new_type = build_distinct_type_copy (orig_type);
+      TYPE_ARG_TYPES (new_type) = new_reversed;
+    }
+  else
+    {
+      new_type
+        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
+							 new_reversed));
+      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
+    }
+
+  if (skip_return)
+    TREE_TYPE (new_type) = void_type_node;
+
+  /* This is a new type, not a copy of an old type.  Need to reassociate
+     variants.  We can handle everything except the main variant lazily.  */
+  t = TYPE_MAIN_VARIANT (orig_type);
+  if (t != orig_type)
+    {
+      t = build_function_type_skip_args (t, args_to_skip, skip_return);
+      TYPE_MAIN_VARIANT (new_type) = t;
+      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
+      TYPE_NEXT_VARIANT (t) = new_type;
+    }
+  else
+    {
+      TYPE_MAIN_VARIANT (new_type) = new_type;
+      TYPE_NEXT_VARIANT (new_type) = NULL;
+    }
+
+  return new_type;
+}
+
+/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
+   return value if SKIP_RETURN is true.
+
+   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
+   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
+   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
+
+static tree
+build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
+			       bool skip_return)
+{
+  tree new_decl = copy_node (orig_decl);
+  tree new_type;
+
+  new_type = TREE_TYPE (orig_decl);
+  if (prototype_p (new_type)
+      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
+    new_type
+      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
+  TREE_TYPE (new_decl) = new_type;
+
+  /* For declarations setting DECL_VINDEX (i.e. methods)
+     we expect first argument to be THIS pointer.   */
+  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
+    DECL_VINDEX (new_decl) = NULL_TREE;
+
+  /* When signature changes, we need to clear builtin info.  */
+  if (DECL_BUILT_IN (new_decl)
+      && args_to_skip
+      && !bitmap_empty_p (args_to_skip))
+    {
+      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
+      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
+    }
+  /* The FE might have information and assumptions about the other
+     arguments.  */
+  DECL_LANG_SPECIFIC (new_decl) = NULL;
+  return new_decl;
+}
+
+/* Duplicate thunk THUNK but make it to refer to NODE.  ARGS_TO_SKIP, if
+   non-NULL, determines which parameters should be omitted.  */
+
+static cgraph_node *
+duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node,
+			  bitmap args_to_skip)
+{
+  cgraph_node *new_thunk, *thunk_of;
+  thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee);
+
+  if (thunk_of->thunk.thunk_p)
+    node = duplicate_thunk_for_node (thunk_of, node, args_to_skip);
+
+  tree new_decl;
+  if (!args_to_skip)
+    new_decl = copy_node (thunk->decl);
+  else
+    new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip, false);
+
+  gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
+  gcc_checking_assert (!DECL_INITIAL (new_decl));
+  gcc_checking_assert (!DECL_RESULT (new_decl));
+  gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
+
+  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
+  SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
+  DECL_EXTERNAL (new_decl) = 0;
+  DECL_SECTION_NAME (new_decl) = NULL;
+  DECL_COMDAT_GROUP (new_decl) = 0;
+  TREE_PUBLIC (new_decl) = 0;
+  DECL_COMDAT (new_decl) = 0;
+  DECL_WEAK (new_decl) = 0;
+  DECL_VIRTUAL_P (new_decl) = 0;
+  DECL_STATIC_CONSTRUCTOR (new_decl) = 0;
+  DECL_STATIC_DESTRUCTOR (new_decl) = 0;
+
+  new_thunk = cgraph_create_node (new_decl);
+  new_thunk->definition = true;
+  new_thunk->thunk = thunk->thunk;
+  new_thunk->unique_name = in_lto_p;
+  new_thunk->externally_visible = 0;
+  new_thunk->local.local = 1;
+  new_thunk->lowered = true;
+  new_thunk->former_clone_of = thunk->decl;
+
+  struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0,
+					      CGRAPH_FREQ_BASE);
+  e->call_stmt_cannot_inline_p = true;
+  cgraph_call_edge_duplication_hooks (thunk->callees, e);
+  if (!expand_thunk (new_thunk, false))
+    new_thunk->analyzed = true;
+  cgraph_call_node_duplication_hooks (thunk, new_thunk);
+  return new_thunk;
+}
+
+/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
+   one or more equivalent thunks for N and redirect E to the first in the
+   chain.  */
+
+void
+redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
+				  bitmap args_to_skip)
+{
+  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
+  if (orig_to->thunk.thunk_p)
+    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
+
+  cgraph_redirect_edge_callee (e, n);
+}
 
 /* Create node representing clone of N executed COUNT times.  Decrease
    the execution counts from original node too.
@@ -190,7 +367,8 @@ cgraph_clone_node (struct cgraph_node *n
 		   bool update_original,
 		   vec<cgraph_edge_p> redirect_callers,
 		   bool call_duplication_hook,
-		   struct cgraph_node *new_inlined_to)
+		   struct cgraph_node *new_inlined_to,
+		   bitmap args_to_skip)
 {
   struct cgraph_node *new_node = cgraph_create_empty_node ();
   struct cgraph_edge *e;
@@ -239,7 +417,7 @@ cgraph_clone_node (struct cgraph_node *n
     {
       /* Redirect calls to the old version node to point to its new
 	 version.  */
-      cgraph_redirect_edge_callee (e, new_node);
+      redirect_edge_duplicating_thunks (e, new_node, args_to_skip);
     }
 
 
@@ -288,114 +466,6 @@ clone_function_name (tree decl, const ch
   return get_identifier (tmp_name);
 }
 
-/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
-   return value if SKIP_RETURN is true.  */
-
-static tree
-build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
-			       bool skip_return)
-{
-  tree new_type = NULL;
-  tree args, new_args = NULL, t;
-  tree new_reversed;
-  int i = 0;
-
-  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
-       args = TREE_CHAIN (args), i++)
-    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
-      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
-
-  new_reversed = nreverse (new_args);
-  if (args)
-    {
-      if (new_reversed)
-        TREE_CHAIN (new_args) = void_list_node;
-      else
-	new_reversed = void_list_node;
-    }
-
-  /* Use copy_node to preserve as much as possible from original type
-     (debug info, attribute lists etc.)
-     Exception is METHOD_TYPEs must have THIS argument.
-     When we are asked to remove it, we need to build new FUNCTION_TYPE
-     instead.  */
-  if (TREE_CODE (orig_type) != METHOD_TYPE
-      || !args_to_skip
-      || !bitmap_bit_p (args_to_skip, 0))
-    {
-      new_type = build_distinct_type_copy (orig_type);
-      TYPE_ARG_TYPES (new_type) = new_reversed;
-    }
-  else
-    {
-      new_type
-        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
-							 new_reversed));
-      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
-    }
-
-  if (skip_return)
-    TREE_TYPE (new_type) = void_type_node;
-
-  /* This is a new type, not a copy of an old type.  Need to reassociate
-     variants.  We can handle everything except the main variant lazily.  */
-  t = TYPE_MAIN_VARIANT (orig_type);
-  if (t != orig_type)
-    {
-      t = build_function_type_skip_args (t, args_to_skip, skip_return);
-      TYPE_MAIN_VARIANT (new_type) = t;
-      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
-      TYPE_NEXT_VARIANT (t) = new_type;
-    }
-  else
-    {
-      TYPE_MAIN_VARIANT (new_type) = new_type;
-      TYPE_NEXT_VARIANT (new_type) = NULL;
-    }
-
-  return new_type;
-}
-
-/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
-   return value if SKIP_RETURN is true.
-
-   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
-   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
-   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
-
-static tree
-build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
-			       bool skip_return)
-{
-  tree new_decl = copy_node (orig_decl);
-  tree new_type;
-
-  new_type = TREE_TYPE (orig_decl);
-  if (prototype_p (new_type)
-      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
-    new_type
-      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
-  TREE_TYPE (new_decl) = new_type;
-
-  /* For declarations setting DECL_VINDEX (i.e. methods)
-     we expect first argument to be THIS pointer.   */
-  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
-    DECL_VINDEX (new_decl) = NULL_TREE;
-
-  /* When signature changes, we need to clear builtin info.  */
-  if (DECL_BUILT_IN (new_decl)
-      && args_to_skip
-      && !bitmap_empty_p (args_to_skip))
-    {
-      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
-      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
-    }
-  /* The FE might have information and assumptions about the other
-     arguments.  */
-  DECL_LANG_SPECIFIC (new_decl) = NULL;
-  return new_decl;
-}
-
 /* Create callgraph node clone with new declaration.  The actual body will
    be copied later at compilation stage.
 
@@ -449,7 +519,7 @@ cgraph_create_virtual_clone (struct cgra
 
   new_node = cgraph_clone_node (old_node, new_decl, old_node->count,
 				CGRAPH_FREQ_BASE, false,
-				redirect_callers, false, NULL);
+				redirect_callers, false, NULL, args_to_skip);
   /* Update the properties.
      Make clone visible only within this translation unit.  Make sure
      that is not weak also.
Index: src/gcc/ipa-inline-transform.c
===================================================================
--- src.orig/gcc/ipa-inline-transform.c
+++ src/gcc/ipa-inline-transform.c
@@ -184,7 +184,7 @@ clone_inlined_nodes (struct cgraph_edge
 	    freq_scale = e->frequency;
 	  n = cgraph_clone_node (e->callee, e->callee->decl,
 				 e->count, freq_scale, update_original,
-				 vNULL, true, inlining_into);
+				 vNULL, true, inlining_into, NULL);
 	  cgraph_redirect_edge_callee (e, n);
 	}
     }
Index: src/gcc/ipa-inline.c
===================================================================
--- src.orig/gcc/ipa-inline.c
+++ src/gcc/ipa-inline.c
@@ -1388,7 +1388,7 @@ recursive_inlining (struct cgraph_edge *
 	  /* We need original clone to copy around.  */
 	  master_clone = cgraph_clone_node (node, node->decl,
 					    node->count, CGRAPH_FREQ_BASE,
-					    false, vNULL, true, NULL);
+					    false, vNULL, true, NULL, NULL);
 	  for (e = master_clone->callees; e; e = e->next_callee)
 	    if (!e->inline_failed)
 	      clone_inlined_nodes (e, true, false, NULL, CGRAPH_FREQ_BASE);
Index: src/gcc/lto-cgraph.c
===================================================================
--- src.orig/gcc/lto-cgraph.c
+++ src/gcc/lto-cgraph.c
@@ -1042,7 +1042,7 @@ input_node (struct lto_file_decl_data *f
     {
       node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl,
 				0, CGRAPH_FREQ_BASE, false,
-				vNULL, false, NULL);
+				vNULL, false, NULL, NULL);
     }
   else
     {
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
@@ -0,0 +1,50 @@
+// { dg-do compile }
+// { dg-options "-O3" }
+
+class ASN1Object
+{
+public:
+  virtual ~ASN1Object ();
+};
+class A
+{
+  virtual unsigned m_fn1 () const;
+};
+class B
+{
+public:
+  ASN1Object Element;
+  virtual unsigned m_fn1 (bool) const;
+};
+template <class BASE> class C : public BASE
+{
+};
+
+class D : ASN1Object, public B
+{
+};
+class G : public D
+{
+  unsigned m_fn1 (bool) const {}
+};
+class F : A
+{
+public:
+  F (A);
+  unsigned m_fn1 () const
+  {
+    int a;
+    a = m_fn2 ().m_fn1 (0);
+    return a;
+  }
+  const B &m_fn2 () const { return m_groupParameters; }
+  C<G> m_groupParameters;
+};
+template <class D> void BenchMarkKeyAgreement (int *, int *, int)
+{
+  A f;
+  D d (f);
+}
+
+void BenchmarkAll2 () { BenchMarkKeyAgreement<F>(0, 0, 0); }
+

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Jan Hubicka]

> Hi,
> 
> this patch fixes PR 60640 by creating thunks to clones when that is
> necessary to properly redirect edges to them.  I mostly does what
> cgraph_add_thunk does and what analyze_function does to thunks.  It
> fixes the testcases on trunk (it does not apply to 4.8, I have not
> looked how easily fixable that it) and passes bootstrap and testing on
> x86_64-linux.
> 
> OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2014-03-26  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraph.h (cgraph_clone_node): New parameter added to declaration.
> 	Adjust all callers.
> 	* cgraphclones.c (build_function_type_skip_args): Moved upwards in the
> 	file.
> 	(build_function_decl_skip_args): Likewise.
> 	(duplicate_thunk_for_node): New function.
> 	(redirect_edge_duplicating_thunks): Likewise.
> 	(cgraph_clone_node): New parameter args_to_skip, pass it to
> 	redirect_edge_duplicating_thunks which is called instead of
> 	cgraph_redirect_edge_callee.
> 	(cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node.
> +/* Duplicate thunk THUNK but make it to refer to NODE.  ARGS_TO_SKIP, if
> +   non-NULL, determines which parameters should be omitted.  */
> +
> +static cgraph_node *
> +duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node,
> +			  bitmap args_to_skip)
> +{
> +  cgraph_node *new_thunk, *thunk_of;
> +  thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee);
> +
> +  if (thunk_of->thunk.thunk_p)
> +    node = duplicate_thunk_for_node (thunk_of, node, args_to_skip);
> +
> +  tree new_decl;
> +  if (!args_to_skip)
> +    new_decl = copy_node (thunk->decl);
> +  else
> +    new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip, false);
> +
> +  gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
> +  gcc_checking_assert (!DECL_INITIAL (new_decl));
> +  gcc_checking_assert (!DECL_RESULT (new_decl));
> +  gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
> +
> +  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
> +  SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
> +  DECL_EXTERNAL (new_decl) = 0;
> +  DECL_SECTION_NAME (new_decl) = NULL;
> +  DECL_COMDAT_GROUP (new_decl) = 0;
> +  TREE_PUBLIC (new_decl) = 0;
> +  DECL_COMDAT (new_decl) = 0;
> +  DECL_WEAK (new_decl) = 0;
> +  DECL_VIRTUAL_P (new_decl) = 0;
> +  DECL_STATIC_CONSTRUCTOR (new_decl) = 0;
> +  DECL_STATIC_DESTRUCTOR (new_decl) = 0;

We probably ought to factor out this to common subfunction.
> +
> +  new_thunk = cgraph_create_node (new_decl);
> +  new_thunk->definition = true;
> +  new_thunk->thunk = thunk->thunk;
> +  new_thunk->unique_name = in_lto_p;
> +  new_thunk->externally_visible = 0;
> +  new_thunk->local.local = 1;
> +  new_thunk->lowered = true;
> +  new_thunk->former_clone_of = thunk->decl;
> +
> +  struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0,
> +					      CGRAPH_FREQ_BASE);
> +  e->call_stmt_cannot_inline_p = true;
> +  cgraph_call_edge_duplication_hooks (thunk->callees, e);
> +  if (!expand_thunk (new_thunk, false))
> +    new_thunk->analyzed = true;
> +  cgraph_call_node_duplication_hooks (thunk, new_thunk);
> +  return new_thunk;
> +}
> +
> +/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
> +   one or more equivalent thunks for N and redirect E to the first in the
> +   chain.  */
> +
> +void
> +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
> +				  bitmap args_to_skip)
> +{
> +  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
> +  if (orig_to->thunk.thunk_p)
> +    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);

Is there anything that would pevent us from creating a new thunk for each call?

Also I think you need to avoid this logic when THIS parameter is being optimized out
(i.e. it is part of skip_args)
Thanks for looking into this!

Honza

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Jan Hubicka]

> Hi,
> 
> this patch fixes PR 60640 by creating thunks to clones when that is
> necessary to properly redirect edges to them.  I mostly does what
> cgraph_add_thunk does and what analyze_function does to thunks.  It
> fixes the testcases on trunk (it does not apply to 4.8, I have not
> looked how easily fixable that it) and passes bootstrap and testing on
> x86_64-linux.
> 
> OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2014-03-26  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraph.h (cgraph_clone_node): New parameter added to declaration.
> 	Adjust all callers.
> 	* cgraphclones.c (build_function_type_skip_args): Moved upwards in the
> 	file.
> 	(build_function_decl_skip_args): Likewise.
> 	(duplicate_thunk_for_node): New function.
> 	(redirect_edge_duplicating_thunks): Likewise.
> 	(cgraph_clone_node): New parameter args_to_skip, pass it to
> 	redirect_edge_duplicating_thunks which is called instead of
> 	cgraph_redirect_edge_callee.
> 	(cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node.
> 
> testsuite/
> 	* g++.dg/ipa/pr60640-1.C: New test.
> 	* g++.dg/ipa/pr60640-2.C: Likewise.
> 
> Index: src/gcc/cgraph.h
> ===================================================================
> --- src.orig/gcc/cgraph.h
> +++ src/gcc/cgraph.h
> @@ -890,7 +890,7 @@ struct cgraph_edge * cgraph_clone_edge (
>  					unsigned, gcov_type, int, bool);
>  struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type,
>  					int, bool, vec<cgraph_edge_p>,
> -					bool, struct cgraph_node *);
> +					bool, struct cgraph_node *, bitmap);
>  tree clone_function_name (tree decl, const char *);
>  struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node,
>  			                          vec<cgraph_edge_p>,
> Index: src/gcc/cgraphclones.c
> ===================================================================
> --- src.orig/gcc/cgraphclones.c
> +++ src/gcc/cgraphclones.c
> @@ -168,6 +168,183 @@ cgraph_clone_edge (struct cgraph_edge *e
>    return new_edge;
>  }
>  
> +/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
> +   return value if SKIP_RETURN is true.  */
> +
> +static tree
> +build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
> +			       bool skip_return)
> +{
> +  tree new_type = NULL;
> +  tree args, new_args = NULL, t;
> +  tree new_reversed;
> +  int i = 0;
> +
> +  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
> +       args = TREE_CHAIN (args), i++)
> +    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
> +      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
> +
> +  new_reversed = nreverse (new_args);
> +  if (args)
> +    {
> +      if (new_reversed)
> +        TREE_CHAIN (new_args) = void_list_node;
> +      else
> +	new_reversed = void_list_node;
> +    }
> +
> +  /* Use copy_node to preserve as much as possible from original type
> +     (debug info, attribute lists etc.)
> +     Exception is METHOD_TYPEs must have THIS argument.
> +     When we are asked to remove it, we need to build new FUNCTION_TYPE
> +     instead.  */
> +  if (TREE_CODE (orig_type) != METHOD_TYPE
> +      || !args_to_skip
> +      || !bitmap_bit_p (args_to_skip, 0))
> +    {
> +      new_type = build_distinct_type_copy (orig_type);
> +      TYPE_ARG_TYPES (new_type) = new_reversed;
> +    }
> +  else
> +    {
> +      new_type
> +        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
> +							 new_reversed));
> +      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
> +    }
> +
> +  if (skip_return)
> +    TREE_TYPE (new_type) = void_type_node;
> +
> +  /* This is a new type, not a copy of an old type.  Need to reassociate
> +     variants.  We can handle everything except the main variant lazily.  */
> +  t = TYPE_MAIN_VARIANT (orig_type);
> +  if (t != orig_type)
> +    {
> +      t = build_function_type_skip_args (t, args_to_skip, skip_return);
> +      TYPE_MAIN_VARIANT (new_type) = t;
> +      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
> +      TYPE_NEXT_VARIANT (t) = new_type;
> +    }
> +  else
> +    {
> +      TYPE_MAIN_VARIANT (new_type) = new_type;
> +      TYPE_NEXT_VARIANT (new_type) = NULL;
> +    }
> +
> +  return new_type;
> +}
> +
> +/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
> +   return value if SKIP_RETURN is true.
> +
> +   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
> +   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
> +   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
> +
> +static tree
> +build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
> +			       bool skip_return)
> +{
> +  tree new_decl = copy_node (orig_decl);
> +  tree new_type;
> +
> +  new_type = TREE_TYPE (orig_decl);
> +  if (prototype_p (new_type)
> +      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
> +    new_type
> +      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
> +  TREE_TYPE (new_decl) = new_type;
> +
> +  /* For declarations setting DECL_VINDEX (i.e. methods)
> +     we expect first argument to be THIS pointer.   */
> +  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
> +    DECL_VINDEX (new_decl) = NULL_TREE;
> +
> +  /* When signature changes, we need to clear builtin info.  */
> +  if (DECL_BUILT_IN (new_decl)
> +      && args_to_skip
> +      && !bitmap_empty_p (args_to_skip))
> +    {
> +      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
> +      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
> +    }
> +  /* The FE might have information and assumptions about the other
> +     arguments.  */
> +  DECL_LANG_SPECIFIC (new_decl) = NULL;
> +  return new_decl;
> +}
> +
> +/* Duplicate thunk THUNK but make it to refer to NODE.  ARGS_TO_SKIP, if
> +   non-NULL, determines which parameters should be omitted.  */
> +
> +static cgraph_node *
> +duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node,
> +			  bitmap args_to_skip)
> +{
> +  cgraph_node *new_thunk, *thunk_of;
> +  thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee);
> +
> +  if (thunk_of->thunk.thunk_p)
> +    node = duplicate_thunk_for_node (thunk_of, node, args_to_skip);
> +
> +  tree new_decl;
> +  if (!args_to_skip)
> +    new_decl = copy_node (thunk->decl);
> +  else
> +    new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip, false);
> +
> +  gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
> +  gcc_checking_assert (!DECL_INITIAL (new_decl));
> +  gcc_checking_assert (!DECL_RESULT (new_decl));
> +  gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
> +
> +  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
> +  SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
> +  DECL_EXTERNAL (new_decl) = 0;
> +  DECL_SECTION_NAME (new_decl) = NULL;
> +  DECL_COMDAT_GROUP (new_decl) = 0;
> +  TREE_PUBLIC (new_decl) = 0;
> +  DECL_COMDAT (new_decl) = 0;
> +  DECL_WEAK (new_decl) = 0;
> +  DECL_VIRTUAL_P (new_decl) = 0;
> +  DECL_STATIC_CONSTRUCTOR (new_decl) = 0;
> +  DECL_STATIC_DESTRUCTOR (new_decl) = 0;
> +
> +  new_thunk = cgraph_create_node (new_decl);
> +  new_thunk->definition = true;
> +  new_thunk->thunk = thunk->thunk;
> +  new_thunk->unique_name = in_lto_p;
> +  new_thunk->externally_visible = 0;
> +  new_thunk->local.local = 1;
> +  new_thunk->lowered = true;
> +  new_thunk->former_clone_of = thunk->decl;
> +
> +  struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0,
> +					      CGRAPH_FREQ_BASE);
> +  e->call_stmt_cannot_inline_p = true;
> +  cgraph_call_edge_duplication_hooks (thunk->callees, e);
> +  if (!expand_thunk (new_thunk, false))
> +    new_thunk->analyzed = true;
> +  cgraph_call_node_duplication_hooks (thunk, new_thunk);
> +  return new_thunk;
> +}
> +
> +/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
> +   one or more equivalent thunks for N and redirect E to the first in the
> +   chain.  */
> +
> +void
> +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
> +				  bitmap args_to_skip)
> +{
> +  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
> +  if (orig_to->thunk.thunk_p)
> +    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
> +
> +  cgraph_redirect_edge_callee (e, n);
> +}
>  
>  /* Create node representing clone of N executed COUNT times.  Decrease
>     the execution counts from original node too.
> @@ -190,7 +367,8 @@ cgraph_clone_node (struct cgraph_node *n
>  		   bool update_original,
>  		   vec<cgraph_edge_p> redirect_callers,
>  		   bool call_duplication_hook,
> -		   struct cgraph_node *new_inlined_to)
> +		   struct cgraph_node *new_inlined_to,
> +		   bitmap args_to_skip)
>  {
>    struct cgraph_node *new_node = cgraph_create_empty_node ();
>    struct cgraph_edge *e;
> @@ -239,7 +417,7 @@ cgraph_clone_node (struct cgraph_node *n
>      {
>        /* Redirect calls to the old version node to point to its new
>  	 version.  */
> -      cgraph_redirect_edge_callee (e, new_node);
> +      redirect_edge_duplicating_thunks (e, new_node, args_to_skip);
>      }
>  
>  
> @@ -288,114 +466,6 @@ clone_function_name (tree decl, const ch
>    return get_identifier (tmp_name);
>  }
>  
> -/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
> -   return value if SKIP_RETURN is true.  */
> -
> -static tree
> -build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
> -			       bool skip_return)
> -{
> -  tree new_type = NULL;
> -  tree args, new_args = NULL, t;
> -  tree new_reversed;
> -  int i = 0;
> -
> -  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
> -       args = TREE_CHAIN (args), i++)
> -    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
> -      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
> -
> -  new_reversed = nreverse (new_args);
> -  if (args)
> -    {
> -      if (new_reversed)
> -        TREE_CHAIN (new_args) = void_list_node;
> -      else
> -	new_reversed = void_list_node;
> -    }
> -
> -  /* Use copy_node to preserve as much as possible from original type
> -     (debug info, attribute lists etc.)
> -     Exception is METHOD_TYPEs must have THIS argument.
> -     When we are asked to remove it, we need to build new FUNCTION_TYPE
> -     instead.  */
> -  if (TREE_CODE (orig_type) != METHOD_TYPE
> -      || !args_to_skip
> -      || !bitmap_bit_p (args_to_skip, 0))
> -    {
> -      new_type = build_distinct_type_copy (orig_type);
> -      TYPE_ARG_TYPES (new_type) = new_reversed;
> -    }
> -  else
> -    {
> -      new_type
> -        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
> -							 new_reversed));
> -      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
> -    }
> -
> -  if (skip_return)
> -    TREE_TYPE (new_type) = void_type_node;
> -
> -  /* This is a new type, not a copy of an old type.  Need to reassociate
> -     variants.  We can handle everything except the main variant lazily.  */
> -  t = TYPE_MAIN_VARIANT (orig_type);
> -  if (t != orig_type)
> -    {
> -      t = build_function_type_skip_args (t, args_to_skip, skip_return);
> -      TYPE_MAIN_VARIANT (new_type) = t;
> -      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
> -      TYPE_NEXT_VARIANT (t) = new_type;
> -    }
> -  else
> -    {
> -      TYPE_MAIN_VARIANT (new_type) = new_type;
> -      TYPE_NEXT_VARIANT (new_type) = NULL;
> -    }
> -
> -  return new_type;
> -}
> -
> -/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
> -   return value if SKIP_RETURN is true.
> -
> -   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
> -   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
> -   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
> -
> -static tree
> -build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
> -			       bool skip_return)
> -{
> -  tree new_decl = copy_node (orig_decl);
> -  tree new_type;
> -
> -  new_type = TREE_TYPE (orig_decl);
> -  if (prototype_p (new_type)
> -      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
> -    new_type
> -      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
> -  TREE_TYPE (new_decl) = new_type;
> -
> -  /* For declarations setting DECL_VINDEX (i.e. methods)
> -     we expect first argument to be THIS pointer.   */
> -  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
> -    DECL_VINDEX (new_decl) = NULL_TREE;
> -
> -  /* When signature changes, we need to clear builtin info.  */
> -  if (DECL_BUILT_IN (new_decl)
> -      && args_to_skip
> -      && !bitmap_empty_p (args_to_skip))
> -    {
> -      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
> -      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
> -    }
> -  /* The FE might have information and assumptions about the other
> -     arguments.  */
> -  DECL_LANG_SPECIFIC (new_decl) = NULL;
> -  return new_decl;
> -}
> -
>  /* Create callgraph node clone with new declaration.  The actual body will
>     be copied later at compilation stage.
>  
> @@ -449,7 +519,7 @@ cgraph_create_virtual_clone (struct cgra
>  
>    new_node = cgraph_clone_node (old_node, new_decl, old_node->count,
>  				CGRAPH_FREQ_BASE, false,
> -				redirect_callers, false, NULL);
> +				redirect_callers, false, NULL, args_to_skip);
>    /* Update the properties.
>       Make clone visible only within this translation unit.  Make sure
>       that is not weak also.
> Index: src/gcc/ipa-inline-transform.c
> ===================================================================
> --- src.orig/gcc/ipa-inline-transform.c
> +++ src/gcc/ipa-inline-transform.c
> @@ -184,7 +184,7 @@ clone_inlined_nodes (struct cgraph_edge
>  	    freq_scale = e->frequency;
>  	  n = cgraph_clone_node (e->callee, e->callee->decl,
>  				 e->count, freq_scale, update_original,
> -				 vNULL, true, inlining_into);
> +				 vNULL, true, inlining_into, NULL);
>  	  cgraph_redirect_edge_callee (e, n);
>  	}
>      }
> Index: src/gcc/ipa-inline.c
> ===================================================================
> --- src.orig/gcc/ipa-inline.c
> +++ src/gcc/ipa-inline.c
> @@ -1388,7 +1388,7 @@ recursive_inlining (struct cgraph_edge *
>  	  /* We need original clone to copy around.  */
>  	  master_clone = cgraph_clone_node (node, node->decl,
>  					    node->count, CGRAPH_FREQ_BASE,
> -					    false, vNULL, true, NULL);
> +					    false, vNULL, true, NULL, NULL);
>  	  for (e = master_clone->callees; e; e = e->next_callee)
>  	    if (!e->inline_failed)
>  	      clone_inlined_nodes (e, true, false, NULL, CGRAPH_FREQ_BASE);
> Index: src/gcc/lto-cgraph.c
> ===================================================================
> --- src.orig/gcc/lto-cgraph.c
> +++ src/gcc/lto-cgraph.c
> @@ -1042,7 +1042,7 @@ input_node (struct lto_file_decl_data *f
>      {
>        node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl,
>  				0, CGRAPH_FREQ_BASE, false,
> -				vNULL, false, NULL);
> +				vNULL, false, NULL, NULL);
>      }
>    else
>      {
> Index: src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
> ===================================================================
> --- /dev/null
> +++ src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
> @@ -0,0 +1,50 @@
> +// { dg-do compile }
> +// { dg-options "-O3" }
> +
> +class ASN1Object
> +{
> +public:
> +  virtual ~ASN1Object ();
> +};
> +class A
> +{
> +  virtual unsigned m_fn1 () const;
> +};
> +class B
> +{
> +public:
> +  ASN1Object Element;
> +  virtual unsigned m_fn1 (bool) const;
> +};
> +template <class BASE> class C : public BASE
> +{
> +};
> +
> +class D : ASN1Object, public B
> +{
> +};
> +class G : public D
> +{
> +  unsigned m_fn1 (bool) const {}
> +};
> +class F : A
> +{
> +public:
> +  F (A);
> +  unsigned m_fn1 () const
> +  {
> +    int a;
> +    a = m_fn2 ().m_fn1 (0);
> +    return a;
> +  }
> +  const B &m_fn2 () const { return m_groupParameters; }
> +  C<G> m_groupParameters;
> +};
> +template <class D> void BenchMarkKeyAgreement (int *, int *, int)
> +{
> +  A f;
> +  D d (f);
> +}
> +
> +void BenchmarkAll2 () { BenchMarkKeyAgreement<F>(0, 0, 0); }
> +

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Martin Jambor]

Hi,

On Fri, Mar 28, 2014 at 09:43:53PM +0100, Jan Hubicka wrote:
> > Hi,
> > 
> > this patch fixes PR 60640 by creating thunks to clones when that is
> > necessary to properly redirect edges to them.  I mostly does what
> > cgraph_add_thunk does and what analyze_function does to thunks.  It
> > fixes the testcases on trunk (it does not apply to 4.8, I have not
> > looked how easily fixable that it) and passes bootstrap and testing on
> > x86_64-linux.
> > 
> > OK for trunk?
> > 
> > Thanks,
> > 
> > Martin
> > 
> > 
> > 2014-03-26  Martin Jambor  <mjambor@suse.cz>
> > 
> > 	* cgraph.h (cgraph_clone_node): New parameter added to declaration.
> > 	Adjust all callers.
> > 	* cgraphclones.c (build_function_type_skip_args): Moved upwards in the
> > 	file.
> > 	(build_function_decl_skip_args): Likewise.
> > 	(duplicate_thunk_for_node): New function.
> > 	(redirect_edge_duplicating_thunks): Likewise.
> > 	(cgraph_clone_node): New parameter args_to_skip, pass it to
> > 	redirect_edge_duplicating_thunks which is called instead of
> > 	cgraph_redirect_edge_callee.
> > 	(cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node.
> > +/* Duplicate thunk THUNK but make it to refer to NODE.  ARGS_TO_SKIP, if
> > +   non-NULL, determines which parameters should be omitted.  */
> > +
> > +static cgraph_node *
> > +duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node,
> > +			  bitmap args_to_skip)
> > +{
> > +  cgraph_node *new_thunk, *thunk_of;
> > +  thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee);
> > +
> > +  if (thunk_of->thunk.thunk_p)
> > +    node = duplicate_thunk_for_node (thunk_of, node, args_to_skip);
> > +
> > +  tree new_decl;
> > +  if (!args_to_skip)
> > +    new_decl = copy_node (thunk->decl);
> > +  else
> > +    new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip, false);
> > +
> > +  gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
> > +  gcc_checking_assert (!DECL_INITIAL (new_decl));
> > +  gcc_checking_assert (!DECL_RESULT (new_decl));
> > +  gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
> > +
> > +  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
> > +  SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
> > +  DECL_EXTERNAL (new_decl) = 0;
> > +  DECL_SECTION_NAME (new_decl) = NULL;
> > +  DECL_COMDAT_GROUP (new_decl) = 0;
> > +  TREE_PUBLIC (new_decl) = 0;
> > +  DECL_COMDAT (new_decl) = 0;
> > +  DECL_WEAK (new_decl) = 0;
> > +  DECL_VIRTUAL_P (new_decl) = 0;
> > +  DECL_STATIC_CONSTRUCTOR (new_decl) = 0;
> > +  DECL_STATIC_DESTRUCTOR (new_decl) = 0;
> 
> We probably ought to factor out this to common subfunction.

I did that in the new patch below.

> > +
> > +  new_thunk = cgraph_create_node (new_decl);
> > +  new_thunk->definition = true;
> > +  new_thunk->thunk = thunk->thunk;
> > +  new_thunk->unique_name = in_lto_p;
> > +  new_thunk->externally_visible = 0;
> > +  new_thunk->local.local = 1;
> > +  new_thunk->lowered = true;
> > +  new_thunk->former_clone_of = thunk->decl;
> > +
> > +  struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0,
> > +					      CGRAPH_FREQ_BASE);
> > +  e->call_stmt_cannot_inline_p = true;
> > +  cgraph_call_edge_duplication_hooks (thunk->callees, e);
> > +  if (!expand_thunk (new_thunk, false))
> > +    new_thunk->analyzed = true;
> > +  cgraph_call_node_duplication_hooks (thunk, new_thunk);
> > +  return new_thunk;
> > +}
> > +
> > +/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
> > +   one or more equivalent thunks for N and redirect E to the first in the
> > +   chain.  */
> > +
> > +void
> > +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
> > +				  bitmap args_to_skip)
> > +{
> > +  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
> > +  if (orig_to->thunk.thunk_p)
> > +    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
> 
> Is there anything that would pevent us from creating a new thunk for
> each call?

No, given how late we have discovered it, it probably only happens
very rarely.  Moreover, since you have plans to always inline only
directly called thunks for the next release, which should be the
ultimate solution, I did not think it was necessary or even
appropriate at this stage.

> 
> Also I think you need to avoid this logic when THIS parameter is being optimized out
> (i.e. it is part of skip_args)

You are of course right.  However, skipping the creation of a new
thunk when we are also removing parameter this leads to verification
errors again, so I had to also teach the verifier that this case is
actually OK.  Moreover, although it seems that currently all
non-this_adjusting thunks are expanded before IPA-CP runs, I made sure
the skipping logic checked that flag.

Accidently, the two original testcases are removing parameter this so
I added a new one, which also shows how current trunk miscompiles
stuff.  Unfortunately, at the moment it relies on speculative edges
and so when IPA-CP correctly redirects calls to a thunk, inlining
gives up and removes the edge, so the IPA-CP transformation is not
run-time checked.  However, the cgraph verifier does see the edge
before that happens and is OK with it.

I have also took the liberty of removing an extra call to
cgraph_function_or_thunk_node (clone_of_p calls it too) and a clearly
obsolete comment from verify_edge_corresponds_to_fndecl.

Bootstrapped and tested on x86_64-linux.  OK for trunk?

Thanks,

Martin


2014-03-31  Martin Jambor  <mjambor@suse.cz>

        * cgraph.h (cgraph_clone_node): New parameter added to declaration.
        Adjust all callers.
	* cgraph.c (clone_of_p): Also return true if thunks match.
	(verify_edge_corresponds_to_fndecl): Removed extraneous call to
	cgraph_function_or_thunk_node and an obsolete comment.
        * cgraphclones.c (build_function_type_skip_args): Moved upwards in the
        file.
        (build_function_decl_skip_args): Likewise.
	(set_new_clone_decl_and_node_flags): New function.
        (duplicate_thunk_for_node): Likewise.
        (redirect_edge_duplicating_thunks): Likewise.
        (cgraph_clone_node): New parameter args_to_skip, pass it to
        redirect_edge_duplicating_thunks which is called instead of
        cgraph_redirect_edge_callee.
        (cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node,
	moved setting of a lot of flags to set_new_clone_decl_and_node_flags.

testsuite/
        * g++.dg/ipa/pr60640-1.C: New test.
        * g++.dg/ipa/pr60640-2.C: Likewise.
        * g++.dg/ipa/pr60640-3.C: Likewise.

Index: src/gcc/cgraph.h
===================================================================
--- src.orig/gcc/cgraph.h
+++ src/gcc/cgraph.h
@@ -890,7 +890,7 @@ struct cgraph_edge * cgraph_clone_edge (
 					unsigned, gcov_type, int, bool);
 struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type,
 					int, bool, vec<cgraph_edge_p>,
-					bool, struct cgraph_node *);
+					bool, struct cgraph_node *, bitmap);
 tree clone_function_name (tree decl, const char *);
 struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node,
 			                          vec<cgraph_edge_p>,
Index: src/gcc/cgraphclones.c
===================================================================
--- src.orig/gcc/cgraphclones.c
+++ src/gcc/cgraphclones.c
@@ -168,6 +168,203 @@ cgraph_clone_edge (struct cgraph_edge *e
   return new_edge;
 }
 
+/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
+   return value if SKIP_RETURN is true.  */
+
+static tree
+build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
+			       bool skip_return)
+{
+  tree new_type = NULL;
+  tree args, new_args = NULL, t;
+  tree new_reversed;
+  int i = 0;
+
+  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
+       args = TREE_CHAIN (args), i++)
+    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
+      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
+
+  new_reversed = nreverse (new_args);
+  if (args)
+    {
+      if (new_reversed)
+        TREE_CHAIN (new_args) = void_list_node;
+      else
+	new_reversed = void_list_node;
+    }
+
+  /* Use copy_node to preserve as much as possible from original type
+     (debug info, attribute lists etc.)
+     Exception is METHOD_TYPEs must have THIS argument.
+     When we are asked to remove it, we need to build new FUNCTION_TYPE
+     instead.  */
+  if (TREE_CODE (orig_type) != METHOD_TYPE
+      || !args_to_skip
+      || !bitmap_bit_p (args_to_skip, 0))
+    {
+      new_type = build_distinct_type_copy (orig_type);
+      TYPE_ARG_TYPES (new_type) = new_reversed;
+    }
+  else
+    {
+      new_type
+        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
+							 new_reversed));
+      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
+    }
+
+  if (skip_return)
+    TREE_TYPE (new_type) = void_type_node;
+
+  /* This is a new type, not a copy of an old type.  Need to reassociate
+     variants.  We can handle everything except the main variant lazily.  */
+  t = TYPE_MAIN_VARIANT (orig_type);
+  if (t != orig_type)
+    {
+      t = build_function_type_skip_args (t, args_to_skip, skip_return);
+      TYPE_MAIN_VARIANT (new_type) = t;
+      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
+      TYPE_NEXT_VARIANT (t) = new_type;
+    }
+  else
+    {
+      TYPE_MAIN_VARIANT (new_type) = new_type;
+      TYPE_NEXT_VARIANT (new_type) = NULL;
+    }
+
+  return new_type;
+}
+
+/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
+   return value if SKIP_RETURN is true.
+
+   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
+   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
+   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
+
+static tree
+build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
+			       bool skip_return)
+{
+  tree new_decl = copy_node (orig_decl);
+  tree new_type;
+
+  new_type = TREE_TYPE (orig_decl);
+  if (prototype_p (new_type)
+      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
+    new_type
+      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
+  TREE_TYPE (new_decl) = new_type;
+
+  /* For declarations setting DECL_VINDEX (i.e. methods)
+     we expect first argument to be THIS pointer.   */
+  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
+    DECL_VINDEX (new_decl) = NULL_TREE;
+
+  /* When signature changes, we need to clear builtin info.  */
+  if (DECL_BUILT_IN (new_decl)
+      && args_to_skip
+      && !bitmap_empty_p (args_to_skip))
+    {
+      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
+      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
+    }
+  /* The FE might have information and assumptions about the other
+     arguments.  */
+  DECL_LANG_SPECIFIC (new_decl) = NULL;
+  return new_decl;
+}
+
+/* Set flags of NEW_NODE and its decl.  NEW_NODE is a newly created private
+   clone or its thunk.  */
+
+static void
+set_new_clone_decl_and_node_flags (cgraph_node *new_node)
+{
+  DECL_EXTERNAL (new_node->decl) = 0;
+  DECL_COMDAT_GROUP (new_node->decl) = 0;
+  TREE_PUBLIC (new_node->decl) = 0;
+  DECL_COMDAT (new_node->decl) = 0;
+  DECL_WEAK (new_node->decl) = 0;
+  DECL_VIRTUAL_P (new_node->decl) = 0;
+  DECL_STATIC_CONSTRUCTOR (new_node->decl) = 0;
+  DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
+
+  new_node->externally_visible = 0;
+  new_node->local.local = 1;
+  new_node->lowered = true;
+}
+
+/* Duplicate thunk THUNK if necessary but make it to refer to NODE.
+   ARGS_TO_SKIP, if non-NULL, determines which parameters should be omitted.
+   Function can return NODE if no thunk is necessary, which can happen when
+   thunk is this_adjusting but we are removing this parameter.  */
+
+static cgraph_node *
+duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node,
+			  bitmap args_to_skip)
+{
+  cgraph_node *new_thunk, *thunk_of;
+  thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee);
+
+  if (thunk_of->thunk.thunk_p)
+    node = duplicate_thunk_for_node (thunk_of, node, args_to_skip);
+
+  tree new_decl;
+  if (!args_to_skip)
+    new_decl = copy_node (thunk->decl);
+  else
+    {
+      /* We do not need to duplicate this_adjusting thunks if we have removed
+	 this.  */
+      if (thunk->thunk.this_adjusting
+	  && bitmap_bit_p (args_to_skip, 0))
+	return node;
+
+      new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip,
+						false);
+    }
+  gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
+  gcc_checking_assert (!DECL_INITIAL (new_decl));
+  gcc_checking_assert (!DECL_RESULT (new_decl));
+  gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
+
+  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
+  SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
+  DECL_SECTION_NAME (new_decl) = NULL;
+
+  new_thunk = cgraph_create_node (new_decl);
+  set_new_clone_decl_and_node_flags (new_thunk);
+  new_thunk->definition = true;
+  new_thunk->thunk = thunk->thunk;
+  new_thunk->unique_name = in_lto_p;
+  new_thunk->former_clone_of = thunk->decl;
+
+  struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0,
+					      CGRAPH_FREQ_BASE);
+  e->call_stmt_cannot_inline_p = true;
+  cgraph_call_edge_duplication_hooks (thunk->callees, e);
+  if (!expand_thunk (new_thunk, false))
+    new_thunk->analyzed = true;
+  cgraph_call_node_duplication_hooks (thunk, new_thunk);
+  return new_thunk;
+}
+
+/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
+   one or more equivalent thunks for N and redirect E to the first in the
+   chain.  */
+
+void
+redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
+				  bitmap args_to_skip)
+{
+  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
+  if (orig_to->thunk.thunk_p)
+    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
+
+  cgraph_redirect_edge_callee (e, n);
+}
 
 /* Create node representing clone of N executed COUNT times.  Decrease
    the execution counts from original node too.
@@ -190,7 +387,8 @@ cgraph_clone_node (struct cgraph_node *n
 		   bool update_original,
 		   vec<cgraph_edge_p> redirect_callers,
 		   bool call_duplication_hook,
-		   struct cgraph_node *new_inlined_to)
+		   struct cgraph_node *new_inlined_to,
+		   bitmap args_to_skip)
 {
   struct cgraph_node *new_node = cgraph_create_empty_node ();
   struct cgraph_edge *e;
@@ -243,7 +441,7 @@ cgraph_clone_node (struct cgraph_node *n
       if (!e->callee
 	  || DECL_BUILT_IN_CLASS (e->callee->decl) != BUILT_IN_NORMAL
 	  || DECL_FUNCTION_CODE (e->callee->decl) != BUILT_IN_UNREACHABLE)
-        cgraph_redirect_edge_callee (e, new_node);
+        redirect_edge_duplicating_thunks (e, new_node, args_to_skip);
     }
 
 
@@ -292,114 +490,6 @@ clone_function_name (tree decl, const ch
   return get_identifier (tmp_name);
 }
 
-/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
-   return value if SKIP_RETURN is true.  */
-
-static tree
-build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
-			       bool skip_return)
-{
-  tree new_type = NULL;
-  tree args, new_args = NULL, t;
-  tree new_reversed;
-  int i = 0;
-
-  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
-       args = TREE_CHAIN (args), i++)
-    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
-      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
-
-  new_reversed = nreverse (new_args);
-  if (args)
-    {
-      if (new_reversed)
-        TREE_CHAIN (new_args) = void_list_node;
-      else
-	new_reversed = void_list_node;
-    }
-
-  /* Use copy_node to preserve as much as possible from original type
-     (debug info, attribute lists etc.)
-     Exception is METHOD_TYPEs must have THIS argument.
-     When we are asked to remove it, we need to build new FUNCTION_TYPE
-     instead.  */
-  if (TREE_CODE (orig_type) != METHOD_TYPE
-      || !args_to_skip
-      || !bitmap_bit_p (args_to_skip, 0))
-    {
-      new_type = build_distinct_type_copy (orig_type);
-      TYPE_ARG_TYPES (new_type) = new_reversed;
-    }
-  else
-    {
-      new_type
-        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
-							 new_reversed));
-      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
-    }
-
-  if (skip_return)
-    TREE_TYPE (new_type) = void_type_node;
-
-  /* This is a new type, not a copy of an old type.  Need to reassociate
-     variants.  We can handle everything except the main variant lazily.  */
-  t = TYPE_MAIN_VARIANT (orig_type);
-  if (t != orig_type)
-    {
-      t = build_function_type_skip_args (t, args_to_skip, skip_return);
-      TYPE_MAIN_VARIANT (new_type) = t;
-      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
-      TYPE_NEXT_VARIANT (t) = new_type;
-    }
-  else
-    {
-      TYPE_MAIN_VARIANT (new_type) = new_type;
-      TYPE_NEXT_VARIANT (new_type) = NULL;
-    }
-
-  return new_type;
-}
-
-/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
-   return value if SKIP_RETURN is true.
-
-   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
-   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
-   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
-
-static tree
-build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
-			       bool skip_return)
-{
-  tree new_decl = copy_node (orig_decl);
-  tree new_type;
-
-  new_type = TREE_TYPE (orig_decl);
-  if (prototype_p (new_type)
-      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
-    new_type
-      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
-  TREE_TYPE (new_decl) = new_type;
-
-  /* For declarations setting DECL_VINDEX (i.e. methods)
-     we expect first argument to be THIS pointer.   */
-  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
-    DECL_VINDEX (new_decl) = NULL_TREE;
-
-  /* When signature changes, we need to clear builtin info.  */
-  if (DECL_BUILT_IN (new_decl)
-      && args_to_skip
-      && !bitmap_empty_p (args_to_skip))
-    {
-      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
-      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
-    }
-  /* The FE might have information and assumptions about the other
-     arguments.  */
-  DECL_LANG_SPECIFIC (new_decl) = NULL;
-  return new_decl;
-}
-
 /* Create callgraph node clone with new declaration.  The actual body will
    be copied later at compilation stage.
 
@@ -453,22 +543,15 @@ cgraph_create_virtual_clone (struct cgra
 
   new_node = cgraph_clone_node (old_node, new_decl, old_node->count,
 				CGRAPH_FREQ_BASE, false,
-				redirect_callers, false, NULL);
+				redirect_callers, false, NULL, args_to_skip);
   /* Update the properties.
      Make clone visible only within this translation unit.  Make sure
      that is not weak also.
      ??? We cannot use COMDAT linkage because there is no
      ABI support for this.  */
-  DECL_EXTERNAL (new_node->decl) = 0;
   if (DECL_ONE_ONLY (old_decl))
     DECL_SECTION_NAME (new_node->decl) = NULL;
-  DECL_COMDAT_GROUP (new_node->decl) = 0;
-  TREE_PUBLIC (new_node->decl) = 0;
-  DECL_COMDAT (new_node->decl) = 0;
-  DECL_WEAK (new_node->decl) = 0;
-  DECL_VIRTUAL_P (new_node->decl) = 0;
-  DECL_STATIC_CONSTRUCTOR (new_node->decl) = 0;
-  DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
+  set_new_clone_decl_and_node_flags (new_node);
   new_node->clone.tree_map = tree_map;
   new_node->clone.args_to_skip = args_to_skip;
 
@@ -508,9 +591,6 @@ cgraph_create_virtual_clone (struct cgra
     }
   else
     new_node->clone.combined_args_to_skip = args_to_skip;
-  new_node->externally_visible = 0;
-  new_node->local.local = 1;
-  new_node->lowered = true;
 
   cgraph_call_node_duplication_hooks (old_node, new_node);
 
Index: src/gcc/ipa-inline-transform.c
===================================================================
--- src.orig/gcc/ipa-inline-transform.c
+++ src/gcc/ipa-inline-transform.c
@@ -184,7 +184,7 @@ clone_inlined_nodes (struct cgraph_edge
 	    freq_scale = e->frequency;
 	  n = cgraph_clone_node (e->callee, e->callee->decl,
 				 e->count, freq_scale, update_original,
-				 vNULL, true, inlining_into);
+				 vNULL, true, inlining_into, NULL);
 	  cgraph_redirect_edge_callee (e, n);
 	}
     }
Index: src/gcc/ipa-inline.c
===================================================================
--- src.orig/gcc/ipa-inline.c
+++ src/gcc/ipa-inline.c
@@ -1383,7 +1383,7 @@ recursive_inlining (struct cgraph_edge *
 	  /* We need original clone to copy around.  */
 	  master_clone = cgraph_clone_node (node, node->decl,
 					    node->count, CGRAPH_FREQ_BASE,
-					    false, vNULL, true, NULL);
+					    false, vNULL, true, NULL, NULL);
 	  for (e = master_clone->callees; e; e = e->next_callee)
 	    if (!e->inline_failed)
 	      clone_inlined_nodes (e, true, false, NULL, CGRAPH_FREQ_BASE);
Index: src/gcc/lto-cgraph.c
===================================================================
--- src.orig/gcc/lto-cgraph.c
+++ src/gcc/lto-cgraph.c
@@ -1042,7 +1042,7 @@ input_node (struct lto_file_decl_data *f
     {
       node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl,
 				0, CGRAPH_FREQ_BASE, false,
-				vNULL, false, NULL);
+				vNULL, false, NULL, NULL);
     }
   else
     {
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
@@ -0,0 +1,50 @@
+// { dg-do compile }
+// { dg-options "-O3" }
+
+class ASN1Object
+{
+public:
+  virtual ~ASN1Object ();
+};
+class A
+{
+  virtual unsigned m_fn1 () const;
+};
+class B
+{
+public:
+  ASN1Object Element;
+  virtual unsigned m_fn1 (bool) const;
+};
+template <class BASE> class C : public BASE
+{
+};
+
+class D : ASN1Object, public B
+{
+};
+class G : public D
+{
+  unsigned m_fn1 (bool) const {}
+};
+class F : A
+{
+public:
+  F (A);
+  unsigned m_fn1 () const
+  {
+    int a;
+    a = m_fn2 ().m_fn1 (0);
+    return a;
+  }
+  const B &m_fn2 () const { return m_groupParameters; }
+  C<G> m_groupParameters;
+};
+template <class D> void BenchMarkKeyAgreement (int *, int *, int)
+{
+  A f;
+  D d (f);
+}
+
+void BenchmarkAll2 () { BenchMarkKeyAgreement<F>(0, 0, 0); }
+
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-2.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-2.C
@@ -0,0 +1,15 @@
+// { dg-do compile }
+// { dg-options "-O3" }
+
+struct B { virtual unsigned f () const; };
+struct C { virtual void f (); };
+struct F { virtual unsigned f (bool) const; ~F (); };
+struct J : C, F {};
+struct G : J { unsigned f (bool) const { return 0; } };
+struct H : B
+{
+  H (int);
+  unsigned f () const { return ((const F &) h).f (0); }
+  G h;
+};
+H h (0);
Index: src/gcc/cgraph.c
===================================================================
--- src.orig/gcc/cgraph.c
+++ src/gcc/cgraph.c
@@ -2543,12 +2543,34 @@ collect_callers_of_node (struct cgraph_n
   return redirect_callers;
 }
 
-/* Return TRUE if NODE2 is equivalent to NODE or its clone.  */
+/* Return TRUE if NODE2 a clone of NODE or is equivalent to it.  */
+
 static bool
 clone_of_p (struct cgraph_node *node, struct cgraph_node *node2)
 {
+  bool skipped_thunk = false;
   node = cgraph_function_or_thunk_node (node, NULL);
   node2 = cgraph_function_or_thunk_node (node2, NULL);
+
+  /* There are no virtual clones of thunks so check former_clone_of or if we
+     might have skipped thunks because this adjustments are no longer
+     necessary.  */
+  while (node->thunk.thunk_p)
+    {
+      if (node2->former_clone_of == node->decl)
+	return true;
+      if (!node->thunk.this_adjusting)
+	return false;
+      node = cgraph_function_or_thunk_node (node->callees->callee, NULL);
+      skipped_thunk = true;
+    }
+
+  if (skipped_thunk
+      && (!node2->clone_of
+	  || !node2->clone.args_to_skip
+	  || !bitmap_bit_p (node2->clone.args_to_skip, 0)))
+    return false;
+
   while (node != node2 && node2)
     node2 = node2->clone_of;
   return node2 != NULL;
@@ -2648,10 +2670,8 @@ verify_edge_corresponds_to_fndecl (struc
   node = cgraph_function_or_thunk_node (node, NULL);
 
   if (e->callee->former_clone_of != node->decl
-      /* IPA-CP sometimes redirect edge to clone and then back to the former
-	 function.  This ping-pong has to go, eventually.  */
       && (node != cgraph_function_or_thunk_node (e->callee, NULL))
-      && !clone_of_p (cgraph_function_or_thunk_node (node, NULL), e->callee))
+      && !clone_of_p (node, e->callee))
     return true;
   else
     return false;
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-3.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-3.C
@@ -0,0 +1,81 @@
+// { dg-do run }
+// { dg-options "-O3" }
+
+struct Distraction
+{
+  char fc[8];
+  virtual Distraction * return_self ()
+  { return this; }
+};
+
+namespace {
+
+struct A;
+static A * __attribute__ ((noinline, noclone)) get_an_A ();
+
+static int go;
+
+struct A
+{
+  int fi;
+
+  A () : fi(777) {}
+  A (int pi) : fi (pi) {}
+  virtual A * foo (int p) = 0;
+};
+
+struct B;
+static B * __attribute__ ((noinline, noclone)) get_a_B ();
+
+struct B : public Distraction, A
+{
+  B () : Distraction(), A() { }
+  B (int pi) : Distraction (), A (pi) {}
+  virtual B * foo (int p)
+  {
+    int o = fi;
+    for (int i = 0; i < p; i++)
+      o += i + i * i;
+    go = o;
+
+    return get_a_B ();
+  }
+};
+
+
+struct B gb1 (1111), gb2 (2);
+static B * __attribute__ ((noinline, noclone))
+get_a_B ()
+{
+  return &gb1;
+}
+
+static A * __attribute__ ((noinline, noclone))
+get_an_A ()
+{
+  return &gb2;
+}
+
+}
+
+static int __attribute__ ((noinline, noclone))
+get_a_number ()
+{
+  return 5;
+}
+
+extern "C" void abort (void);
+
+int main (int argc, char *argv[])
+{
+  for (int i = 0; i < get_a_number (); i++)
+    {
+      struct A *p = get_an_A ();
+      struct A *r = p->foo (4);
+      if (r->fi != 1111)
+	abort ();
+      if (go != 22)
+	abort ();
+    }
+  return 0;
+}

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Jan Hubicka]

> > > +/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
> > > +   one or more equivalent thunks for N and redirect E to the first in the
> > > +   chain.  */
> > > +
> > > +void
> > > +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
> > > +				  bitmap args_to_skip)
> > > +{
> > > +  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
> > > +  if (orig_to->thunk.thunk_p)
> > > +    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
> > 
> > Is there anything that would pevent us from creating a new thunk for
> > each call?
> 
> No, given how late we have discovered it, it probably only happens
> very rarely.  Moreover, since you have plans to always inline only
> directly called thunks for the next release, which should be the
> ultimate solution, I did not think it was necessary or even
> appropriate at this stage.

A lot of code iterate over thunks/aliases and expect this to be cheap operation.
We thus need to be sure we won't create very many thunks or aliases of a given
function internally.

In order to trigger quadratic behaviour here, we only need a single function
call used very often in a big project, like mozilla, to create uncontrolled
numbers of thunks.  I would suggest to just walk existing thunks before
creating new looking if there is one mathcing our needs.  Same code is in
making local aliases. This change is pre-approved.
> 
> > 
> > Also I think you need to avoid this logic when THIS parameter is being optimized out
> > (i.e. it is part of skip_args)
> 
> You are of course right.  However, skipping the creation of a new
> thunk when we are also removing parameter this leads to verification
> errors again, so I had to also teach the verifier that this case is
> actually OK.  Moreover, although it seems that currently all
That is fine with me.
> non-this_adjusting thunks are expanded before IPA-CP runs, I made sure
> the skipping logic checked that flag.

Yes, we only keep the simple thunks in non-lowered form, but I do not
see how it makes difference for you.
> 
> Accidently, the two original testcases are removing parameter this so
> I added a new one, which also shows how current trunk miscompiles
> stuff.  Unfortunately, at the moment it relies on speculative edges
> and so when IPA-CP correctly redirects calls to a thunk, inlining
> gives up and removes the edge, so the IPA-CP transformation is not
> run-time checked.  However, the cgraph verifier does see the edge
> before that happens and is OK with it.

You can probably play with anonymous namespaces and final flags to get
it devirtualized unconditnally.
> 
> I have also took the liberty of removing an extra call to
> cgraph_function_or_thunk_node (clone_of_p calls it too) and a clearly
> obsolete comment from verify_edge_corresponds_to_fndecl.
> 
> Bootstrapped and tested on x86_64-linux.  OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2014-03-31  Martin Jambor  <mjambor@suse.cz>
> 
>         * cgraph.h (cgraph_clone_node): New parameter added to declaration.
>         Adjust all callers.
> 	* cgraph.c (clone_of_p): Also return true if thunks match.
> 	(verify_edge_corresponds_to_fndecl): Removed extraneous call to
> 	cgraph_function_or_thunk_node and an obsolete comment.
>         * cgraphclones.c (build_function_type_skip_args): Moved upwards in the
>         file.
>         (build_function_decl_skip_args): Likewise.
> 	(set_new_clone_decl_and_node_flags): New function.
>         (duplicate_thunk_for_node): Likewise.
>         (redirect_edge_duplicating_thunks): Likewise.
>         (cgraph_clone_node): New parameter args_to_skip, pass it to
>         redirect_edge_duplicating_thunks which is called instead of
>         cgraph_redirect_edge_callee.
>         (cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node,
> 	moved setting of a lot of flags to set_new_clone_decl_and_node_flags.
> 
> testsuite/
>         * g++.dg/ipa/pr60640-1.C: New test.
>         * g++.dg/ipa/pr60640-2.C: Likewise.
>         * g++.dg/ipa/pr60640-3.C: Likewise.

OK, with the change above.

Honza

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Martin Jambor]

Hi,

On Thu, Apr 03, 2014 at 11:19:10PM +0200, Jan Hubicka wrote:
> > > > +/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
> > > > +   one or more equivalent thunks for N and redirect E to the first in the
> > > > +   chain.  */
> > > > +
> > > > +void
> > > > +redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
> > > > +				  bitmap args_to_skip)
> > > > +{
> > > > +  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
> > > > +  if (orig_to->thunk.thunk_p)
> > > > +    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
> > > 
> > > Is there anything that would pevent us from creating a new thunk for
> > > each call?
> > 
> > No, given how late we have discovered it, it probably only happens
> > very rarely.  Moreover, since you have plans to always inline only
> > directly called thunks for the next release, which should be the
> > ultimate solution, I did not think it was necessary or even
> > appropriate at this stage.
> 
> A lot of code iterate over thunks/aliases and expect this to be cheap operation.
> We thus need to be sure we won't create very many thunks or aliases of a given
> function internally.
> 
> In order to trigger quadratic behaviour here, we only need a single function
> call used very often in a big project, like mozilla, to create uncontrolled
> numbers of thunks.  I would suggest to just walk existing thunks before
> creating new looking if there is one mathcing our needs.  Same code is in
> making local aliases. This change is pre-approved.

OK, I have updated the patch.  I have also added one more testcase to
check we do not create extra thunks.

> > 
> > > 
> > > Also I think you need to avoid this logic when THIS parameter is being optimized out
> > > (i.e. it is part of skip_args)
> > 
> > You are of course right.  However, skipping the creation of a new
> > thunk when we are also removing parameter this leads to verification
> > errors again, so I had to also teach the verifier that this case is
> > actually OK.  Moreover, although it seems that currently all
> That is fine with me.
> > non-this_adjusting thunks are expanded before IPA-CP runs, I made sure
> > the skipping logic checked that flag.
> 
> Yes, we only keep the simple thunks in non-lowered form, but I do not
> see how it makes difference for you.
> > 
> > Accidently, the two original testcases are removing parameter this so
> > I added a new one, which also shows how current trunk miscompiles
> > stuff.  Unfortunately, at the moment it relies on speculative edges
> > and so when IPA-CP correctly redirects calls to a thunk, inlining
> > gives up and removes the edge, so the IPA-CP transformation is not
> > run-time checked.  However, the cgraph verifier does see the edge
> > before that happens and is OK with it.
> 
> You can probably play with anonymous namespaces and final flags to get
> it devirtualized unconditnally.

The third testcase uses anonymous namespaces and ipa-devirt correctly
reports that its list of possible targets is final, but even though
the list has only two items and one of them is __cxa_pure_virtual, it
still only devirtualizes speculatively.

> > 
> > I have also took the liberty of removing an extra call to
> > cgraph_function_or_thunk_node (clone_of_p calls it too) and a clearly
> > obsolete comment from verify_edge_corresponds_to_fndecl.
> > 
> > Bootstrapped and tested on x86_64-linux.  OK for trunk?
> > 
> > Thanks,
> > 
> > Martin
> > 
> > 
> > 2014-03-31  Martin Jambor  <mjambor@suse.cz>
> > 
> >         * cgraph.h (cgraph_clone_node): New parameter added to declaration.
> >         Adjust all callers.
> > 	* cgraph.c (clone_of_p): Also return true if thunks match.
> > 	(verify_edge_corresponds_to_fndecl): Removed extraneous call to
> > 	cgraph_function_or_thunk_node and an obsolete comment.
> >         * cgraphclones.c (build_function_type_skip_args): Moved upwards in the
> >         file.
> >         (build_function_decl_skip_args): Likewise.
> > 	(set_new_clone_decl_and_node_flags): New function.
> >         (duplicate_thunk_for_node): Likewise.
> >         (redirect_edge_duplicating_thunks): Likewise.
> >         (cgraph_clone_node): New parameter args_to_skip, pass it to
> >         redirect_edge_duplicating_thunks which is called instead of
> >         cgraph_redirect_edge_callee.
> >         (cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node,
> > 	moved setting of a lot of flags to set_new_clone_decl_and_node_flags.
> > 
> > testsuite/
> >         * g++.dg/ipa/pr60640-1.C: New test.
> >         * g++.dg/ipa/pr60640-2.C: Likewise.
> >         * g++.dg/ipa/pr60640-3.C: Likewise.
> 
> OK, with the change above.
> 

Thanks, this is what I am going to commit, after another round of
bootstrap and testing on x86_64-linux and LTO building Firefox (at
-O2).

Thanks,

Martin


2014-04-04  Martin Jambor  <mjambor@suse.cz>

	PR ipa/60640
        * cgraph.h (cgraph_clone_node): New parameter added to declaration.
        Adjust all callers.
	* cgraph.c (clone_of_p): Also return true if thunks match.
	(verify_edge_corresponds_to_fndecl): Removed extraneous call to
	cgraph_function_or_thunk_node and an obsolete comment.
        * cgraphclones.c (build_function_type_skip_args): Moved upwards in the
        file.
        (build_function_decl_skip_args): Likewise.
	(set_new_clone_decl_and_node_flags): New function.
        (duplicate_thunk_for_node): Likewise.
        (redirect_edge_duplicating_thunks): Likewise.
        (cgraph_clone_node): New parameter args_to_skip, pass it to
        redirect_edge_duplicating_thunks which is called instead of
        cgraph_redirect_edge_callee.
        (cgraph_create_virtual_clone): Pass args_to_skip to cgraph_clone_node,
	moved setting of a lot of flags to set_new_clone_decl_and_node_flags.

testsuite/
        * g++.dg/ipa/pr60640-1.C: New test.
        * g++.dg/ipa/pr60640-2.C: Likewise.
        * g++.dg/ipa/pr60640-3.C: Likewise.
        * g++.dg/ipa/pr60640-4.C: Likewise.

Index: src/gcc/cgraph.h
===================================================================
--- src.orig/gcc/cgraph.h
+++ src/gcc/cgraph.h
@@ -890,7 +890,7 @@ struct cgraph_edge * cgraph_clone_edge (
 					unsigned, gcov_type, int, bool);
 struct cgraph_node * cgraph_clone_node (struct cgraph_node *, tree, gcov_type,
 					int, bool, vec<cgraph_edge_p>,
-					bool, struct cgraph_node *);
+					bool, struct cgraph_node *, bitmap);
 tree clone_function_name (tree decl, const char *);
 struct cgraph_node * cgraph_create_virtual_clone (struct cgraph_node *old_node,
 			                          vec<cgraph_edge_p>,
Index: src/gcc/cgraphclones.c
===================================================================
--- src.orig/gcc/cgraphclones.c
+++ src/gcc/cgraphclones.c
@@ -168,6 +168,212 @@ cgraph_clone_edge (struct cgraph_edge *e
   return new_edge;
 }
 
+/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
+   return value if SKIP_RETURN is true.  */
+
+static tree
+build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
+			       bool skip_return)
+{
+  tree new_type = NULL;
+  tree args, new_args = NULL, t;
+  tree new_reversed;
+  int i = 0;
+
+  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
+       args = TREE_CHAIN (args), i++)
+    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
+      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
+
+  new_reversed = nreverse (new_args);
+  if (args)
+    {
+      if (new_reversed)
+        TREE_CHAIN (new_args) = void_list_node;
+      else
+	new_reversed = void_list_node;
+    }
+
+  /* Use copy_node to preserve as much as possible from original type
+     (debug info, attribute lists etc.)
+     Exception is METHOD_TYPEs must have THIS argument.
+     When we are asked to remove it, we need to build new FUNCTION_TYPE
+     instead.  */
+  if (TREE_CODE (orig_type) != METHOD_TYPE
+      || !args_to_skip
+      || !bitmap_bit_p (args_to_skip, 0))
+    {
+      new_type = build_distinct_type_copy (orig_type);
+      TYPE_ARG_TYPES (new_type) = new_reversed;
+    }
+  else
+    {
+      new_type
+        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
+							 new_reversed));
+      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
+    }
+
+  if (skip_return)
+    TREE_TYPE (new_type) = void_type_node;
+
+  /* This is a new type, not a copy of an old type.  Need to reassociate
+     variants.  We can handle everything except the main variant lazily.  */
+  t = TYPE_MAIN_VARIANT (orig_type);
+  if (t != orig_type)
+    {
+      t = build_function_type_skip_args (t, args_to_skip, skip_return);
+      TYPE_MAIN_VARIANT (new_type) = t;
+      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
+      TYPE_NEXT_VARIANT (t) = new_type;
+    }
+  else
+    {
+      TYPE_MAIN_VARIANT (new_type) = new_type;
+      TYPE_NEXT_VARIANT (new_type) = NULL;
+    }
+
+  return new_type;
+}
+
+/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
+   return value if SKIP_RETURN is true.
+
+   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
+   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
+   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
+
+static tree
+build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
+			       bool skip_return)
+{
+  tree new_decl = copy_node (orig_decl);
+  tree new_type;
+
+  new_type = TREE_TYPE (orig_decl);
+  if (prototype_p (new_type)
+      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
+    new_type
+      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
+  TREE_TYPE (new_decl) = new_type;
+
+  /* For declarations setting DECL_VINDEX (i.e. methods)
+     we expect first argument to be THIS pointer.   */
+  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
+    DECL_VINDEX (new_decl) = NULL_TREE;
+
+  /* When signature changes, we need to clear builtin info.  */
+  if (DECL_BUILT_IN (new_decl)
+      && args_to_skip
+      && !bitmap_empty_p (args_to_skip))
+    {
+      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
+      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
+    }
+  /* The FE might have information and assumptions about the other
+     arguments.  */
+  DECL_LANG_SPECIFIC (new_decl) = NULL;
+  return new_decl;
+}
+
+/* Set flags of NEW_NODE and its decl.  NEW_NODE is a newly created private
+   clone or its thunk.  */
+
+static void
+set_new_clone_decl_and_node_flags (cgraph_node *new_node)
+{
+  DECL_EXTERNAL (new_node->decl) = 0;
+  DECL_COMDAT_GROUP (new_node->decl) = 0;
+  TREE_PUBLIC (new_node->decl) = 0;
+  DECL_COMDAT (new_node->decl) = 0;
+  DECL_WEAK (new_node->decl) = 0;
+  DECL_VIRTUAL_P (new_node->decl) = 0;
+  DECL_STATIC_CONSTRUCTOR (new_node->decl) = 0;
+  DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
+
+  new_node->externally_visible = 0;
+  new_node->local.local = 1;
+  new_node->lowered = true;
+}
+
+/* Duplicate thunk THUNK if necessary but make it to refer to NODE.
+   ARGS_TO_SKIP, if non-NULL, determines which parameters should be omitted.
+   Function can return NODE if no thunk is necessary, which can happen when
+   thunk is this_adjusting but we are removing this parameter.  */
+
+static cgraph_node *
+duplicate_thunk_for_node (cgraph_node *thunk, cgraph_node *node,
+			  bitmap args_to_skip)
+{
+  cgraph_node *new_thunk, *thunk_of;
+  thunk_of = cgraph_function_or_thunk_node (thunk->callees->callee);
+
+  if (thunk_of->thunk.thunk_p)
+    node = duplicate_thunk_for_node (thunk_of, node, args_to_skip);
+
+  struct cgraph_edge *cs;
+  for (cs = node->callers; cs; cs = cs->next_caller)
+    if (cs->caller->thunk.thunk_p
+	&& cs->caller->thunk.this_adjusting == thunk->thunk.this_adjusting
+	&& cs->caller->thunk.fixed_offset == thunk->thunk.fixed_offset
+	&& cs->caller->thunk.virtual_offset_p == thunk->thunk.virtual_offset_p
+	&& cs->caller->thunk.virtual_value == thunk->thunk.virtual_value)
+      return cs->caller;
+
+  tree new_decl;
+  if (!args_to_skip)
+    new_decl = copy_node (thunk->decl);
+  else
+    {
+      /* We do not need to duplicate this_adjusting thunks if we have removed
+	 this.  */
+      if (thunk->thunk.this_adjusting
+	  && bitmap_bit_p (args_to_skip, 0))
+	return node;
+
+      new_decl = build_function_decl_skip_args (thunk->decl, args_to_skip,
+						false);
+    }
+  gcc_checking_assert (!DECL_STRUCT_FUNCTION (new_decl));
+  gcc_checking_assert (!DECL_INITIAL (new_decl));
+  gcc_checking_assert (!DECL_RESULT (new_decl));
+  gcc_checking_assert (!DECL_RTL_SET_P (new_decl));
+
+  DECL_NAME (new_decl) = clone_function_name (thunk->decl, "artificial_thunk");
+  SET_DECL_ASSEMBLER_NAME (new_decl, DECL_NAME (new_decl));
+  DECL_SECTION_NAME (new_decl) = NULL;
+
+  new_thunk = cgraph_create_node (new_decl);
+  set_new_clone_decl_and_node_flags (new_thunk);
+  new_thunk->definition = true;
+  new_thunk->thunk = thunk->thunk;
+  new_thunk->unique_name = in_lto_p;
+  new_thunk->former_clone_of = thunk->decl;
+
+  struct cgraph_edge *e = cgraph_create_edge (new_thunk, node, NULL, 0,
+					      CGRAPH_FREQ_BASE);
+  e->call_stmt_cannot_inline_p = true;
+  cgraph_call_edge_duplication_hooks (thunk->callees, e);
+  if (!expand_thunk (new_thunk, false))
+    new_thunk->analyzed = true;
+  cgraph_call_node_duplication_hooks (thunk, new_thunk);
+  return new_thunk;
+}
+
+/* If E does not lead to a thunk, simply redirect it to N.  Otherwise create
+   one or more equivalent thunks for N and redirect E to the first in the
+   chain.  */
+
+void
+redirect_edge_duplicating_thunks (struct cgraph_edge *e, struct cgraph_node *n,
+				  bitmap args_to_skip)
+{
+  cgraph_node *orig_to = cgraph_function_or_thunk_node (e->callee);
+  if (orig_to->thunk.thunk_p)
+    n = duplicate_thunk_for_node (orig_to, n, args_to_skip);
+
+  cgraph_redirect_edge_callee (e, n);
+}
 
 /* Create node representing clone of N executed COUNT times.  Decrease
    the execution counts from original node too.
@@ -190,7 +396,8 @@ cgraph_clone_node (struct cgraph_node *n
 		   bool update_original,
 		   vec<cgraph_edge_p> redirect_callers,
 		   bool call_duplication_hook,
-		   struct cgraph_node *new_inlined_to)
+		   struct cgraph_node *new_inlined_to,
+		   bitmap args_to_skip)
 {
   struct cgraph_node *new_node = cgraph_create_empty_node ();
   struct cgraph_edge *e;
@@ -243,7 +450,7 @@ cgraph_clone_node (struct cgraph_node *n
       if (!e->callee
 	  || DECL_BUILT_IN_CLASS (e->callee->decl) != BUILT_IN_NORMAL
 	  || DECL_FUNCTION_CODE (e->callee->decl) != BUILT_IN_UNREACHABLE)
-        cgraph_redirect_edge_callee (e, new_node);
+        redirect_edge_duplicating_thunks (e, new_node, args_to_skip);
     }
 
 
@@ -292,114 +499,6 @@ clone_function_name (tree decl, const ch
   return get_identifier (tmp_name);
 }
 
-/* Build variant of function type ORIG_TYPE skipping ARGS_TO_SKIP and the
-   return value if SKIP_RETURN is true.  */
-
-static tree
-build_function_type_skip_args (tree orig_type, bitmap args_to_skip,
-			       bool skip_return)
-{
-  tree new_type = NULL;
-  tree args, new_args = NULL, t;
-  tree new_reversed;
-  int i = 0;
-
-  for (args = TYPE_ARG_TYPES (orig_type); args && args != void_list_node;
-       args = TREE_CHAIN (args), i++)
-    if (!args_to_skip || !bitmap_bit_p (args_to_skip, i))
-      new_args = tree_cons (NULL_TREE, TREE_VALUE (args), new_args);
-
-  new_reversed = nreverse (new_args);
-  if (args)
-    {
-      if (new_reversed)
-        TREE_CHAIN (new_args) = void_list_node;
-      else
-	new_reversed = void_list_node;
-    }
-
-  /* Use copy_node to preserve as much as possible from original type
-     (debug info, attribute lists etc.)
-     Exception is METHOD_TYPEs must have THIS argument.
-     When we are asked to remove it, we need to build new FUNCTION_TYPE
-     instead.  */
-  if (TREE_CODE (orig_type) != METHOD_TYPE
-      || !args_to_skip
-      || !bitmap_bit_p (args_to_skip, 0))
-    {
-      new_type = build_distinct_type_copy (orig_type);
-      TYPE_ARG_TYPES (new_type) = new_reversed;
-    }
-  else
-    {
-      new_type
-        = build_distinct_type_copy (build_function_type (TREE_TYPE (orig_type),
-							 new_reversed));
-      TYPE_CONTEXT (new_type) = TYPE_CONTEXT (orig_type);
-    }
-
-  if (skip_return)
-    TREE_TYPE (new_type) = void_type_node;
-
-  /* This is a new type, not a copy of an old type.  Need to reassociate
-     variants.  We can handle everything except the main variant lazily.  */
-  t = TYPE_MAIN_VARIANT (orig_type);
-  if (t != orig_type)
-    {
-      t = build_function_type_skip_args (t, args_to_skip, skip_return);
-      TYPE_MAIN_VARIANT (new_type) = t;
-      TYPE_NEXT_VARIANT (new_type) = TYPE_NEXT_VARIANT (t);
-      TYPE_NEXT_VARIANT (t) = new_type;
-    }
-  else
-    {
-      TYPE_MAIN_VARIANT (new_type) = new_type;
-      TYPE_NEXT_VARIANT (new_type) = NULL;
-    }
-
-  return new_type;
-}
-
-/* Build variant of function decl ORIG_DECL skipping ARGS_TO_SKIP and the
-   return value if SKIP_RETURN is true.
-
-   Arguments from DECL_ARGUMENTS list can't be removed now, since they are
-   linked by TREE_CHAIN directly.  The caller is responsible for eliminating
-   them when they are being duplicated (i.e. copy_arguments_for_versioning).  */
-
-static tree
-build_function_decl_skip_args (tree orig_decl, bitmap args_to_skip,
-			       bool skip_return)
-{
-  tree new_decl = copy_node (orig_decl);
-  tree new_type;
-
-  new_type = TREE_TYPE (orig_decl);
-  if (prototype_p (new_type)
-      || (skip_return && !VOID_TYPE_P (TREE_TYPE (new_type))))
-    new_type
-      = build_function_type_skip_args (new_type, args_to_skip, skip_return);
-  TREE_TYPE (new_decl) = new_type;
-
-  /* For declarations setting DECL_VINDEX (i.e. methods)
-     we expect first argument to be THIS pointer.   */
-  if (args_to_skip && bitmap_bit_p (args_to_skip, 0))
-    DECL_VINDEX (new_decl) = NULL_TREE;
-
-  /* When signature changes, we need to clear builtin info.  */
-  if (DECL_BUILT_IN (new_decl)
-      && args_to_skip
-      && !bitmap_empty_p (args_to_skip))
-    {
-      DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
-      DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
-    }
-  /* The FE might have information and assumptions about the other
-     arguments.  */
-  DECL_LANG_SPECIFIC (new_decl) = NULL;
-  return new_decl;
-}
-
 /* Create callgraph node clone with new declaration.  The actual body will
    be copied later at compilation stage.
 
@@ -453,22 +552,15 @@ cgraph_create_virtual_clone (struct cgra
 
   new_node = cgraph_clone_node (old_node, new_decl, old_node->count,
 				CGRAPH_FREQ_BASE, false,
-				redirect_callers, false, NULL);
+				redirect_callers, false, NULL, args_to_skip);
   /* Update the properties.
      Make clone visible only within this translation unit.  Make sure
      that is not weak also.
      ??? We cannot use COMDAT linkage because there is no
      ABI support for this.  */
-  DECL_EXTERNAL (new_node->decl) = 0;
   if (DECL_ONE_ONLY (old_decl))
     DECL_SECTION_NAME (new_node->decl) = NULL;
-  DECL_COMDAT_GROUP (new_node->decl) = 0;
-  TREE_PUBLIC (new_node->decl) = 0;
-  DECL_COMDAT (new_node->decl) = 0;
-  DECL_WEAK (new_node->decl) = 0;
-  DECL_VIRTUAL_P (new_node->decl) = 0;
-  DECL_STATIC_CONSTRUCTOR (new_node->decl) = 0;
-  DECL_STATIC_DESTRUCTOR (new_node->decl) = 0;
+  set_new_clone_decl_and_node_flags (new_node);
   new_node->clone.tree_map = tree_map;
   new_node->clone.args_to_skip = args_to_skip;
 
@@ -508,9 +600,6 @@ cgraph_create_virtual_clone (struct cgra
     }
   else
     new_node->clone.combined_args_to_skip = args_to_skip;
-  new_node->externally_visible = 0;
-  new_node->local.local = 1;
-  new_node->lowered = true;
 
   cgraph_call_node_duplication_hooks (old_node, new_node);
 
Index: src/gcc/ipa-inline-transform.c
===================================================================
--- src.orig/gcc/ipa-inline-transform.c
+++ src/gcc/ipa-inline-transform.c
@@ -184,7 +184,7 @@ clone_inlined_nodes (struct cgraph_edge
 	    freq_scale = e->frequency;
 	  n = cgraph_clone_node (e->callee, e->callee->decl,
 				 e->count, freq_scale, update_original,
-				 vNULL, true, inlining_into);
+				 vNULL, true, inlining_into, NULL);
 	  cgraph_redirect_edge_callee (e, n);
 	}
     }
Index: src/gcc/ipa-inline.c
===================================================================
--- src.orig/gcc/ipa-inline.c
+++ src/gcc/ipa-inline.c
@@ -1383,7 +1383,7 @@ recursive_inlining (struct cgraph_edge *
 	  /* We need original clone to copy around.  */
 	  master_clone = cgraph_clone_node (node, node->decl,
 					    node->count, CGRAPH_FREQ_BASE,
-					    false, vNULL, true, NULL);
+					    false, vNULL, true, NULL, NULL);
 	  for (e = master_clone->callees; e; e = e->next_callee)
 	    if (!e->inline_failed)
 	      clone_inlined_nodes (e, true, false, NULL, CGRAPH_FREQ_BASE);
Index: src/gcc/lto-cgraph.c
===================================================================
--- src.orig/gcc/lto-cgraph.c
+++ src/gcc/lto-cgraph.c
@@ -1042,7 +1042,7 @@ input_node (struct lto_file_decl_data *f
     {
       node = cgraph_clone_node (cgraph (nodes[clone_ref]), fn_decl,
 				0, CGRAPH_FREQ_BASE, false,
-				vNULL, false, NULL);
+				vNULL, false, NULL, NULL);
     }
   else
     {
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-1.C
@@ -0,0 +1,50 @@
+// { dg-do compile }
+// { dg-options "-O3" }
+
+class ASN1Object
+{
+public:
+  virtual ~ASN1Object ();
+};
+class A
+{
+  virtual unsigned m_fn1 () const;
+};
+class B
+{
+public:
+  ASN1Object Element;
+  virtual unsigned m_fn1 (bool) const;
+};
+template <class BASE> class C : public BASE
+{
+};
+
+class D : ASN1Object, public B
+{
+};
+class G : public D
+{
+  unsigned m_fn1 (bool) const {}
+};
+class F : A
+{
+public:
+  F (A);
+  unsigned m_fn1 () const
+  {
+    int a;
+    a = m_fn2 ().m_fn1 (0);
+    return a;
+  }
+  const B &m_fn2 () const { return m_groupParameters; }
+  C<G> m_groupParameters;
+};
+template <class D> void BenchMarkKeyAgreement (int *, int *, int)
+{
+  A f;
+  D d (f);
+}
+
+void BenchmarkAll2 () { BenchMarkKeyAgreement<F>(0, 0, 0); }
+
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-2.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-2.C
@@ -0,0 +1,15 @@
+// { dg-do compile }
+// { dg-options "-O3" }
+
+struct B { virtual unsigned f () const; };
+struct C { virtual void f (); };
+struct F { virtual unsigned f (bool) const; ~F (); };
+struct J : C, F {};
+struct G : J { unsigned f (bool) const { return 0; } };
+struct H : B
+{
+  H (int);
+  unsigned f () const { return ((const F &) h).f (0); }
+  G h;
+};
+H h (0);
Index: src/gcc/cgraph.c
===================================================================
--- src.orig/gcc/cgraph.c
+++ src/gcc/cgraph.c
@@ -2543,12 +2543,34 @@ collect_callers_of_node (struct cgraph_n
   return redirect_callers;
 }
 
-/* Return TRUE if NODE2 is equivalent to NODE or its clone.  */
+/* Return TRUE if NODE2 a clone of NODE or is equivalent to it.  */
+
 static bool
 clone_of_p (struct cgraph_node *node, struct cgraph_node *node2)
 {
+  bool skipped_thunk = false;
   node = cgraph_function_or_thunk_node (node, NULL);
   node2 = cgraph_function_or_thunk_node (node2, NULL);
+
+  /* There are no virtual clones of thunks so check former_clone_of or if we
+     might have skipped thunks because this adjustments are no longer
+     necessary.  */
+  while (node->thunk.thunk_p)
+    {
+      if (node2->former_clone_of == node->decl)
+	return true;
+      if (!node->thunk.this_adjusting)
+	return false;
+      node = cgraph_function_or_thunk_node (node->callees->callee, NULL);
+      skipped_thunk = true;
+    }
+
+  if (skipped_thunk
+      && (!node2->clone_of
+	  || !node2->clone.args_to_skip
+	  || !bitmap_bit_p (node2->clone.args_to_skip, 0)))
+    return false;
+
   while (node != node2 && node2)
     node2 = node2->clone_of;
   return node2 != NULL;
@@ -2648,10 +2670,8 @@ verify_edge_corresponds_to_fndecl (struc
   node = cgraph_function_or_thunk_node (node, NULL);
 
   if (e->callee->former_clone_of != node->decl
-      /* IPA-CP sometimes redirect edge to clone and then back to the former
-	 function.  This ping-pong has to go, eventually.  */
       && (node != cgraph_function_or_thunk_node (e->callee, NULL))
-      && !clone_of_p (cgraph_function_or_thunk_node (node, NULL), e->callee))
+      && !clone_of_p (node, e->callee))
     return true;
   else
     return false;
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-3.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-3.C
@@ -0,0 +1,81 @@
+// { dg-do run }
+// { dg-options "-O3" }
+
+struct Distraction
+{
+  char fc[8];
+  virtual Distraction * return_self ()
+  { return this; }
+};
+
+namespace {
+
+struct A;
+static A * __attribute__ ((noinline, noclone)) get_an_A ();
+
+static int go;
+
+struct A
+{
+  int fi;
+
+  A () : fi(777) {}
+  A (int pi) : fi (pi) {}
+  virtual A * foo (int p) = 0;
+};
+
+struct B;
+static B * __attribute__ ((noinline, noclone)) get_a_B ();
+
+struct B : public Distraction, A
+{
+  B () : Distraction(), A() { }
+  B (int pi) : Distraction (), A (pi) {}
+  virtual B * foo (int p)
+  {
+    int o = fi;
+    for (int i = 0; i < p; i++)
+      o += i + i * i;
+    go = o;
+
+    return get_a_B ();
+  }
+};
+
+
+struct B gb1 (1111), gb2 (2);
+static B * __attribute__ ((noinline, noclone))
+get_a_B ()
+{
+  return &gb1;
+}
+
+static A * __attribute__ ((noinline, noclone))
+get_an_A ()
+{
+  return &gb2;
+}
+
+}
+
+static int __attribute__ ((noinline, noclone))
+get_a_number ()
+{
+  return 5;
+}
+
+extern "C" void abort (void);
+
+int main (int argc, char *argv[])
+{
+  for (int i = 0; i < get_a_number (); i++)
+    {
+      struct A *p = get_an_A ();
+      struct A *r = p->foo (4);
+      if (r->fi != 1111)
+	abort ();
+      if (go != 22)
+	abort ();
+    }
+  return 0;
+}
Index: src/gcc/testsuite/g++.dg/ipa/pr60640-4.C
===================================================================
--- /dev/null
+++ src/gcc/testsuite/g++.dg/ipa/pr60640-4.C
@@ -0,0 +1,85 @@
+// { dg-do run }
+// { dg-options "-O3 -fdump-ipa-cp" }
+
+struct Distraction
+{
+  char fc[8];
+  virtual Distraction * return_self ()
+  { return this; }
+};
+
+namespace {
+
+struct A;
+static A * __attribute__ ((noinline, noclone)) get_an_A ();
+
+static int go;
+
+struct A
+{
+  int fi;
+
+  A () : fi(777) {}
+  A (int pi) : fi (pi) {}
+  virtual void foo (int p) = 0;
+};
+
+struct B : public Distraction, A
+{
+  B () : Distraction(), A() { }
+  B (int pi) : Distraction (), A (pi) {}
+  virtual void foo (int p)
+  {
+    int o = fi;
+    for (int i = 0; i < p; i++)
+      o += i + i * i;
+    go = o;
+  }
+};
+
+
+struct B gb (2);
+static A * __attribute__ ((noinline, noclone))
+get_an_A ()
+{
+  return &gb;
+}
+
+}
+
+static int __attribute__ ((noinline, noclone))
+get_a_number ()
+{
+  return 5;
+}
+
+extern "C" void abort (void);
+
+static void __attribute__ ((noinline, noclone))
+bar ()
+{
+  for (int i = 0; i < get_a_number (); i++)
+    {
+      struct A *p = get_an_A ();
+      p->foo (4);
+      if (go != 22)
+	abort ();
+    }
+}
+
+int main (int argc, char *argv[])
+{
+  for (int i = 0; i < get_a_number (); i++)
+    {
+      struct A *p = get_an_A ();
+      p->foo (4);
+      if (go != 22)
+	abort ();
+    }
+
+  bar ();
+  return 0;
+}
+
+/* { dg-final { scan-ipa-dump-times "Thunk fixed offset" 2 "cp"} } */
+/* { dg-final { cleanup-ipa-dump "cp" } } */

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Jan Hubicka]

> The third testcase uses anonymous namespaces and ipa-devirt correctly
> reports that its list of possible targets is final, but even though
> the list has only two items and one of them is __cxa_pure_virtual, it
> still only devirtualizes speculatively.

Ah, yes, it is what I was discussing this with Jason, but apparently the
disucssion died out.  According to his comment __cxa_pure_virtual is a
synonymum for undefined behaviour so it may be correct to unconditionally
devirtualize here (changing runtime beaviour from terminate() to random method
call), but at the moment we don't do that.  Somewhere back in my head I have
burned in from C++ lessons that calling pure virtual method should result in
terminate () call, so I am considering __cxa_pure_virtual to be legitimate call
target (as opposed to any non-function or __builtin_unreachable that I drop
from the list).

I would be happy to change this behaviour, since there are quite few extra
cases where we could devirtualize well then.

Honza

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Jason Merrill]

On 04/04/2014 04:05 PM, Jan Hubicka wrote:
> Ah, yes, it is what I was discussing this with Jason, but apparently the
> discussion died out.  According to his comment __cxa_pure_virtual is a
> synonym for undefined behaviour so it may be correct to unconditionally
> devirtualize here (changing runtime behaviour from terminate() to random method
> call), but at the moment we don't do that.  Somewhere back in my head I have
> burned in from C++ lessons that calling pure virtual method should result in
> terminate () call, so I am considering __cxa_pure_virtual to be legitimate call
> target (as opposed to any non-function or __builtin_unreachable that I drop
> from the list).
>
> I would be happy to change this behaviour, since there are quite few extra
> cases where we could devirtualize well then.

It's definitely undefined.

10.4/6:

Member functions can be called from a constructor (or destructor) of an 
abstract class; the effect of making a virtual call (10.3) to a pure 
virtual function directly or indirectly for the object being created (or 
destroyed) from such a constructor (or destructor) is undefined.

Jason

Re: [PATCH, PR 60640] When creating virtual clones, clone thunks too

[Jan Hubicka]

> On 04/04/2014 04:05 PM, Jan Hubicka wrote:
> >Ah, yes, it is what I was discussing this with Jason, but apparently the
> >discussion died out.  According to his comment __cxa_pure_virtual is a
> >synonym for undefined behaviour so it may be correct to unconditionally
> >devirtualize here (changing runtime behaviour from terminate() to random method
> >call), but at the moment we don't do that.  Somewhere back in my head I have
> >burned in from C++ lessons that calling pure virtual method should result in
> >terminate () call, so I am considering __cxa_pure_virtual to be legitimate call
> >target (as opposed to any non-function or __builtin_unreachable that I drop
> >from the list).
> >
> >I would be happy to change this behaviour, since there are quite few extra
> >cases where we could devirtualize well then.
> 
> It's definitely undefined.
> 
> 10.4/6:
> 
> Member functions can be called from a constructor (or destructor) of
> an abstract class; the effect of making a virtual call (10.3) to a
> pure virtual function directly or indirectly for the object being
> created (or destroyed) from such a constructor (or destructor) is
> undefined.

OK, I will change the behaviour then, thanks!

Thanks,
Honza
> 
> Jason