diff mbox

Avoid redundant indirect_info computation during inderct edge cloning

Message ID 20140814222333.GA34087@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Aug. 14, 2014, 10:23 p.m. UTC
Hi,

I get a segafult in decl_maybe_in_construction_p during function versioning.  We have following steps in clone creation (e.g. as in create_version_clone_with_body):
 1. Create function decl
 2. Create clone of cgraph node
 3. Copy function body
After the first step there is no body attached to function and DECL_STRUCT_FUNCTION for new decl is NULL.  It is initialized on the third step.  But on the second step get_polymorphic_call_info may be called for new function; it calls decl_maybe_in_construction_p which assumes DECL_STRUCT_FUNCTION already exists.

I firstly wanted to fix decl_maybe_in_construction_p but then realized cgraph_clone_edge copy indirect_info from the original edge anyway and therefore its computation is not required at all.

Following patch removes redundant indirect_info computation.  Bootstrapped and regtested on linux-x86_64.  Does it look OK for trunk?

Thanks,
Ilya
--
2014-08-15  Ilya Enkovich  <ilya.enkovich@intel.com>

	* cgraph.h (cgraph_node::create_indirect_edge): Add
	compute_indirect_info param.
	* cgraph.c (cgraph_node::create_indirect_edge): Compute
	indirect_info only when it is required.
	* cgraphclones.c (cgraph_clone_edge): Do not recompute
	indirect_info for cloned indirect edge.

Comments

Jan Hubicka Aug. 15, 2014, 9:08 p.m. UTC | #1
> Hi,
> 
> I get a segafult in decl_maybe_in_construction_p during function versioning.  We have following steps in clone creation (e.g. as in create_version_clone_with_body):
>  1. Create function decl
>  2. Create clone of cgraph node
>  3. Copy function body
> After the first step there is no body attached to function and DECL_STRUCT_FUNCTION for new decl is NULL.  It is initialized on the third step.  But on the second step get_polymorphic_call_info may be called for new function; it calls decl_maybe_in_construction_p which assumes DECL_STRUCT_FUNCTION already exists.
> 
> I firstly wanted to fix decl_maybe_in_construction_p but then realized cgraph_clone_edge copy indirect_info from the original edge anyway and therefore its computation is not required at all.
> 
> Following patch removes redundant indirect_info computation.  Bootstrapped and regtested on linux-x86_64.  Does it look OK for trunk?

OK, plase also add testcase from https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61800

Thanks,
Honza
> 
> Thanks,
> Ilya
> --
> 2014-08-15  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* cgraph.h (cgraph_node::create_indirect_edge): Add
> 	compute_indirect_info param.
> 	* cgraph.c (cgraph_node::create_indirect_edge): Compute
> 	indirect_info only when it is required.
> 	* cgraphclones.c (cgraph_clone_edge): Do not recompute
> 	indirect_info for cloned indirect edge.
> 
> 
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 370a96a..cb49cdc 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -942,7 +942,8 @@ cgraph_allocate_init_indirect_info (void)
>  
>  struct cgraph_edge *
>  cgraph_node::create_indirect_edge (gimple call_stmt, int ecf_flags,
> -				   gcov_type count, int freq)
> +				   gcov_type count, int freq,
> +				   bool compute_indirect_info)
>  {
>    struct cgraph_edge *edge = cgraph_node::create_edge (this, NULL, call_stmt,
>  						       count, freq, true);
> @@ -954,7 +955,8 @@ cgraph_node::create_indirect_edge (gimple call_stmt, int ecf_flags,
>    edge->indirect_info->ecf_flags = ecf_flags;
>  
>    /* Record polymorphic call info.  */
> -  if (call_stmt
> +  if (compute_indirect_info
> +      && call_stmt
>        && (target = gimple_call_fn (call_stmt))
>        && virtual_method_call_p (target))
>      {
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 13c09af..2594ae5 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -915,7 +915,8 @@ public:
>       statement destination is a formal parameter of the caller with index
>       PARAM_INDEX. */
>    struct cgraph_edge *create_indirect_edge (gimple call_stmt, int ecf_flags,
> -					    gcov_type count, int freq);
> +					    gcov_type count, int freq,
> +					    bool compute_indirect_info = true);
>  
>    /* 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
> diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
> index c04b5c8..557f734 100644
> --- a/gcc/cgraphclones.c
> +++ b/gcc/cgraphclones.c
> @@ -136,7 +136,7 @@ cgraph_clone_edge (struct cgraph_edge *e, struct cgraph_node *n,
>  	{
>  	  new_edge = n->create_indirect_edge (call_stmt,
>  					      e->indirect_info->ecf_flags,
> -					      count, freq);
> +					      count, freq, false);
>  	  *new_edge->indirect_info = *e->indirect_info;
>  	}
>      }
diff mbox

Patch

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 370a96a..cb49cdc 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -942,7 +942,8 @@  cgraph_allocate_init_indirect_info (void)
 
 struct cgraph_edge *
 cgraph_node::create_indirect_edge (gimple call_stmt, int ecf_flags,
-				   gcov_type count, int freq)
+				   gcov_type count, int freq,
+				   bool compute_indirect_info)
 {
   struct cgraph_edge *edge = cgraph_node::create_edge (this, NULL, call_stmt,
 						       count, freq, true);
@@ -954,7 +955,8 @@  cgraph_node::create_indirect_edge (gimple call_stmt, int ecf_flags,
   edge->indirect_info->ecf_flags = ecf_flags;
 
   /* Record polymorphic call info.  */
-  if (call_stmt
+  if (compute_indirect_info
+      && call_stmt
       && (target = gimple_call_fn (call_stmt))
       && virtual_method_call_p (target))
     {
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 13c09af..2594ae5 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -915,7 +915,8 @@  public:
      statement destination is a formal parameter of the caller with index
      PARAM_INDEX. */
   struct cgraph_edge *create_indirect_edge (gimple call_stmt, int ecf_flags,
-					    gcov_type count, int freq);
+					    gcov_type count, int freq,
+					    bool compute_indirect_info = true);
 
   /* 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
diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c
index c04b5c8..557f734 100644
--- a/gcc/cgraphclones.c
+++ b/gcc/cgraphclones.c
@@ -136,7 +136,7 @@  cgraph_clone_edge (struct cgraph_edge *e, struct cgraph_node *n,
 	{
 	  new_edge = n->create_indirect_edge (call_stmt,
 					      e->indirect_info->ecf_flags,
-					      count, freq);
+					      count, freq, false);
 	  *new_edge->indirect_info = *e->indirect_info;
 	}
     }