[PATCH] Do not check call type compatibility when cloning cgraph edges

[PATCH] Do not check call type compatibility when cloning cgraph edges

[Martin Jambor]

Hi,

when looking into PR 70929 and thus looking at what
gimple_check_call_matching_types and gimple_check_call_args (both in
cgraph.c) do, I noticed that they get invoked a lot when cloning cgraph
edges when code is being copied as part of inlining transformation, only
to have their result overwritten by the value from the original edge.
(They also fail a lot during that time because since call redirection has
not taken place yet, the arguments are actually expected not to match.)

The following patch avoids that by adding a simple parameter to various
create_edge methods which indicate that we are cloning and the call
statement should therefore not be examined.  For consistency reasons I
unfortunately also had to change the meaning of the last parameter of
create_indirect_edge but there is only one place where it is called with
value of the parameter specified and its intent has always been to
behave differently when cloning.

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

Thanks,

Martin



2019-09-27  Martin Jambor  <mjambor@suse.cz>

	* cgraph.c (symbol_table::create_edge): New parameter cloning_p,
	do not compute some stuff when set.
	(cgraph_node::create_edge): Likewise.
	(cgraph_node::create_indirect_edge): Renamed last parameter to
	coning_p and flipped its meaning, don't even calculate
	inline_failed when set.
	* cgraph.h (cgraph_node::create_edge): Add new parameter.
	(symbol_table::::create_edge): Likewise.
	(cgraph_node::create_indirect_edge): Rename last parameter, flip
	the default value.
	* cgraphclones.c (cgraph_edge::clone): Pass true cloning_p to all
	call graph edge creating functions.
---
 gcc/cgraph.c       | 48 ++++++++++++++++++++++++++++------------------
 gcc/cgraph.h       | 12 +++++++-----
 gcc/cgraphclones.c |  6 +++---
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 331b363c175..c9eb565a9dd 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -824,12 +824,13 @@ cgraph_edge::set_call_stmt (gcall *new_stmt, bool update_speculative)
 
 /* Allocate a cgraph_edge structure and fill it with data according to the
    parameters of which only CALLEE can be NULL (when creating an indirect call
-   edge).  */
+   edge).  CLONING_P should be set if properties that are copied from an
+   original edge should not be calculated.  */
 
 cgraph_edge *
 symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
 			   gcall *call_stmt, profile_count count,
-			   bool indir_unknown_callee)
+			   bool indir_unknown_callee, bool cloning_p)
 {
   cgraph_edge *edge;
 
@@ -862,8 +863,17 @@ symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
   edge->lto_stmt_uid = 0;
 
   edge->count = count;
-
   edge->call_stmt = call_stmt;
+  edge->indirect_info = NULL;
+  edge->indirect_inlining_edge = 0;
+  edge->speculative = false;
+  edge->indirect_unknown_callee = indir_unknown_callee;
+  if (call_stmt && caller->call_site_hash)
+    cgraph_add_edge_to_call_site_hash (edge);
+
+  if (cloning_p)
+    return edge;
+
   edge->can_throw_external
     = call_stmt ? stmt_can_throw_external (DECL_STRUCT_FUNCTION (caller->decl),
 					   call_stmt) : false;
@@ -881,10 +891,6 @@ symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
       edge->call_stmt_cannot_inline_p = false;
     }
 
-  edge->indirect_info = NULL;
-  edge->indirect_inlining_edge = 0;
-  edge->speculative = false;
-  edge->indirect_unknown_callee = indir_unknown_callee;
   if (opt_for_fn (edge->caller->decl, flag_devirtualize)
       && call_stmt && DECL_STRUCT_FUNCTION (caller->decl))
     edge->in_polymorphic_cdtor
@@ -892,22 +898,23 @@ symbol_table::create_edge (cgraph_node *caller, cgraph_node *callee,
 				      caller->decl);
   else
     edge->in_polymorphic_cdtor = caller->thunk.thunk_p;
-  if (call_stmt && caller->call_site_hash)
-    cgraph_add_edge_to_call_site_hash (edge);
 
   return edge;
 }
 
-/* Create edge from a given function to CALLEE in the cgraph.  */
+/* Create edge from a given function to CALLEE in the cgraph.  CLONING_P should
+   be set if properties that are copied from an original edge should not be
+   calculated.  */
 
 cgraph_edge *
 cgraph_node::create_edge (cgraph_node *callee,
-			  gcall *call_stmt, profile_count count)
+			  gcall *call_stmt, profile_count count, bool cloning_p)
 {
   cgraph_edge *edge = symtab->create_edge (this, callee, call_stmt, count,
-					   false);
+					   false, cloning_p);
 
-  initialize_inline_failed (edge);
+  if (!cloning_p)
+    initialize_inline_failed (edge);
 
   edge->next_caller = callee->callers;
   if (callee->callers)
@@ -935,25 +942,28 @@ cgraph_allocate_init_indirect_info (void)
 
 /* Create an indirect edge with a yet-undetermined callee where the call
    statement destination is a formal parameter of the caller with index
-   PARAM_INDEX. */
+   PARAM_INDEX. CLONING_P should be set if properties that are copied from an
+   original edge should not be calculated and indirect_info structure should
+   not be calculated.  */
 
 cgraph_edge *
 cgraph_node::create_indirect_edge (gcall *call_stmt, int ecf_flags,
 				   profile_count count,
-				   bool compute_indirect_info)
+				   bool cloning_p)
 {
-  cgraph_edge *edge = symtab->create_edge (this, NULL, call_stmt,
-							    count, true);
+  cgraph_edge *edge = symtab->create_edge (this, NULL, call_stmt, count, true,
+					   cloning_p);
   tree target;
 
-  initialize_inline_failed (edge);
+  if (!cloning_p)
+    initialize_inline_failed (edge);
 
   edge->indirect_info = cgraph_allocate_init_indirect_info ();
   edge->indirect_info->ecf_flags = ecf_flags;
   edge->indirect_info->vptr_changed = true;
 
   /* Record polymorphic call info.  */
-  if (compute_indirect_info
+  if (!cloning_p
       && call_stmt
       && (target = gimple_call_fn (call_stmt))
       && virtual_method_call_p (target))
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 1da6cab54b0..fd6961a2579 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1161,14 +1161,15 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node : public symtab_node
 
   /* Create edge from a given function to CALLEE in the cgraph.  */
   cgraph_edge *create_edge (cgraph_node *callee,
-			    gcall *call_stmt, profile_count count);
+			    gcall *call_stmt, profile_count count,
+			    bool cloning_p = false);
 
   /* Create an indirect edge with a yet-undetermined callee where the call
      statement destination is a formal parameter of the caller with index
      PARAM_INDEX. */
   cgraph_edge *create_indirect_edge (gcall *call_stmt, int ecf_flags,
 				     profile_count count,
-				     bool compute_indirect_info = true);
+				     bool cloning_p = false);
 
   /* Like cgraph_create_edge walk the clone tree and update all clones sharing
    same function body.  If clones already have edge for OLD_STMT; only
@@ -2381,11 +2382,12 @@ private:
   inline cgraph_node * allocate_cgraph_symbol (void);
 
   /* Allocate a cgraph_edge structure and fill it with data according to the
-     parameters of which only CALLEE can be NULL (when creating an indirect call
-     edge).  */
+     parameters of which only CALLEE can be NULL (when creating an indirect
+     call edge).  CLONING_P should be set if properties that are copied from an
+     original edge should not be calculated.  */
   cgraph_edge *create_edge (cgraph_node *caller, cgraph_node *callee,
 			    gcall *call_stmt, profile_count count,
-			    bool indir_unknown_callee);
+			    bool indir_unknown_callee, bool cloning_p);
 
   /* Put the edge onto the free list.  */
   void free_edge (cgraph_edge *e);
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index 909407b9a71..087b5a26280 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -104,19 +104,19 @@ cgraph_edge::clone (cgraph_node *n, gcall *call_stmt, unsigned stmt_uid,
 	{
 	  cgraph_node *callee = cgraph_node::get (decl);
 	  gcc_checking_assert (callee);
-	  new_edge = n->create_edge (callee, call_stmt, prof_count);
+	  new_edge = n->create_edge (callee, call_stmt, prof_count, true);
 	}
       else
 	{
 	  new_edge = n->create_indirect_edge (call_stmt,
 					      indirect_info->ecf_flags,
-					      prof_count, false);
+					      prof_count, true);
 	  *new_edge->indirect_info = *indirect_info;
 	}
     }
   else
     {
-      new_edge = n->create_edge (callee, call_stmt, prof_count);
+      new_edge = n->create_edge (callee, call_stmt, prof_count, true);
       if (indirect_info)
 	{
 	  new_edge->indirect_info
-- 
2.23.0

Re: [PATCH] Do not check call type compatibility when cloning cgraph edges

[Jeff Law]

On 9/30/19 3:30 AM, Martin Jambor wrote:
> Hi,
> 
> when looking into PR 70929 and thus looking at what
> gimple_check_call_matching_types and gimple_check_call_args (both in
> cgraph.c) do, I noticed that they get invoked a lot when cloning cgraph
> edges when code is being copied as part of inlining transformation, only
> to have their result overwritten by the value from the original edge.
> (They also fail a lot during that time because since call redirection has
> not taken place yet, the arguments are actually expected not to match.)
> 
> The following patch avoids that by adding a simple parameter to various
> create_edge methods which indicate that we are cloning and the call
> statement should therefore not be examined.  For consistency reasons I
> unfortunately also had to change the meaning of the last parameter of
> create_indirect_edge but there is only one place where it is called with
> value of the parameter specified and its intent has always been to
> behave differently when cloning.
> 
> Bootstrapped and tested on x86_64-linux.  OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 
> 2019-09-27  Martin Jambor  <mjambor@suse.cz>
> 
> 	* cgraph.c (symbol_table::create_edge): New parameter cloning_p,
> 	do not compute some stuff when set.
> 	(cgraph_node::create_edge): Likewise.
> 	(cgraph_node::create_indirect_edge): Renamed last parameter to
> 	coning_p and flipped its meaning, don't even calculate
> 	inline_failed when set.
> 	* cgraph.h (cgraph_node::create_edge): Add new parameter.
> 	(symbol_table::::create_edge): Likewise.
> 	(cgraph_node::create_indirect_edge): Rename last parameter, flip
> 	the default value.
> 	* cgraphclones.c (cgraph_edge::clone): Pass true cloning_p to all
> 	call graph edge creating functions.
OK
jeff
> ---