diff mbox

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

Message ID 20140401130015.GX19304@virgil.suse
State New
Headers show

Commit Message

Martin Jambor April 1, 2014, 1 p.m. UTC 
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.

Comments

Jan Hubicka April 3, 2014, 9:19 p.m. UTC | #1 
> > > +/* 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
diff mbox

Patch

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