| Message ID | 3690602f9779a8e66f5744ff22eb2bbd@ispras.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] ipa: fix dumping with deleted multiversioning nodes | expand |
On 09/18/2017 11:08 AM, Evgeny Kudryashov wrote: > Hello, > > The code below causes an internal compiler error in cc1plus (trunk on x86-64) if it is compiled with -fdump-ipa-cgraph. > > int foo () __attribute__ ((target ("default"))); > int foo () __attribute__ ((target ("sse4.2"))); > > __attribute__ ((target ("sse4.2"))) > int foo () > { > return 1; > } > > The error occurs in cgraph_node::dump (gcc/cgraph.c:2065), particularly, in the following fragment: > > cgraph_function_version_info *vi = function_version (); > if (vi != NULL) > { > fprintf (f, " Version info: "); > if (vi->prev != NULL) > { > fprintf (f, "prev: "); > fprintf (f, "%s ", vi->prev->this_node->dump_asm_name ()); > } > if (vi->next != NULL) > { > fprintf (f, "next: "); > fprintf (f, "%s ", vi->next->this_node->dump_asm_name ()); > } > if (vi->dispatcher_resolver != NULL_TREE) > fprintf (f, "dispatcher: %s", > lang_hooks.decl_printable_name (vi->dispatcher_resolver, 2)); > > fprintf (f, "\n"); > } > > > The expression "vi->{prev,next}->this_node" can be null if it is a version of an unused symbol that was removed. > > Is it intentional that removing a cgraph node does not also remove versions? > > As a solution I suggest to delete the version of the node too during node removal. Hi. After reading and testing your patch, I think it's proper fix. Note that I'm not reviewer, but I'm adding Honza who is. Thanks, Martin > > Regards, > Evgeny. > > * cgraph.c (delete_function_version): New, broken out from... > (cgraph_node::delete_function_version): ...here. Rename to > cgraph_node::delete_function_version_by_decl. Update all uses. > (cgraph_node::remove): Call delete_function_version.
> On 09/18/2017 11:08 AM, Evgeny Kudryashov wrote: > > Hello, > > > > The code below causes an internal compiler error in cc1plus (trunk on x86-64) if it is compiled with -fdump-ipa-cgraph. > > > > int foo () __attribute__ ((target ("default"))); > > int foo () __attribute__ ((target ("sse4.2"))); > > > > __attribute__ ((target ("sse4.2"))) > > int foo () > > { > > return 1; > > } > > > > The error occurs in cgraph_node::dump (gcc/cgraph.c:2065), particularly, in the following fragment: > > > > cgraph_function_version_info *vi = function_version (); > > if (vi != NULL) > > { > > fprintf (f, " Version info: "); > > if (vi->prev != NULL) > > { > > fprintf (f, "prev: "); > > fprintf (f, "%s ", vi->prev->this_node->dump_asm_name ()); > > } > > if (vi->next != NULL) > > { > > fprintf (f, "next: "); > > fprintf (f, "%s ", vi->next->this_node->dump_asm_name ()); > > } > > if (vi->dispatcher_resolver != NULL_TREE) > > fprintf (f, "dispatcher: %s", > > lang_hooks.decl_printable_name (vi->dispatcher_resolver, 2)); > > > > fprintf (f, "\n"); > > } > > > > > > The expression "vi->{prev,next}->this_node" can be null if it is a version of an unused symbol that was removed. > > > > Is it intentional that removing a cgraph node does not also remove versions? > > > > As a solution I suggest to delete the version of the node too during node removal. > > Hi. > > After reading and testing your patch, I think it's proper fix. Note that I'm not reviewer, but I'm adding Honza > who is. OK,, thanks! Honza > > Thanks, > Martin > > > > > Regards, > > Evgeny. > > > > * cgraph.c (delete_function_version): New, broken out from... > > (cgraph_node::delete_function_version): ...here. Rename to > > cgraph_node::delete_function_version_by_decl. Update all uses. > > (cgraph_node::remove): Call delete_function_version.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 8bffdec..3d0cefb 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -190,30 +190,34 @@ cgraph_node::insert_new_function_version (void) return version_info_node; } -/* Remove the cgraph_function_version_info and cgraph_node for DECL. This - DECL is a duplicate declaration. */ -void -cgraph_node::delete_function_version (tree decl) +/* Remove the cgraph_function_version_info node given by DECL_V. */ +static void +delete_function_version (cgraph_function_version_info *decl_v) { - cgraph_node *decl_node = cgraph_node::get (decl); - cgraph_function_version_info *decl_v = NULL; - - if (decl_node == NULL) - return; - - decl_v = decl_node->function_version (); - if (decl_v == NULL) return; if (decl_v->prev != NULL) - decl_v->prev->next = decl_v->next; + decl_v->prev->next = decl_v->next; if (decl_v->next != NULL) decl_v->next->prev = decl_v->prev; if (cgraph_fnver_htab != NULL) cgraph_fnver_htab->remove_elt (decl_v); +} + +/* Remove the cgraph_function_version_info and cgraph_node for DECL. This + DECL is a duplicate declaration. */ +void +cgraph_node::delete_function_version_by_decl (tree decl) +{ + cgraph_node *decl_node = cgraph_node::get (decl); + + if (decl_node == NULL) + return; + + delete_function_version (decl_node->function_version ()); decl_node->remove (); } @@ -1844,6 +1848,7 @@ cgraph_node::remove (void) remove_callers (); remove_callees (); ipa_transforms_to_apply.release (); + delete_function_version (function_version ()); /* Incremental inlining access removed nodes stored in the postorder list. */ diff --git a/gcc/cgraph.h b/gcc/cgraph.h index c668b37..1303db0 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -1272,7 +1272,7 @@ public: /* Remove the cgraph_function_version_info and cgraph_node for DECL. This DECL is a duplicate declaration. */ - static void delete_function_version (tree decl); + static void delete_function_version_by_decl (tree decl); /* Add the function FNDECL to the call graph. Unlike finalize_function, this function is intended to be used diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 858747e..50fa1ba 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -2566,7 +2566,7 @@ next_arg:; DECL_FUNCTION_VERSIONED (newdecl) = 1; /* newdecl will be purged after copying to olddecl and is no longer a version. */ - cgraph_node::delete_function_version (newdecl); + cgraph_node::delete_function_version_by_decl (newdecl); } if (TREE_CODE (newdecl) == FUNCTION_DECL)
Hello, The code below causes an internal compiler error in cc1plus (trunk on x86-64) if it is compiled with -fdump-ipa-cgraph. int foo () __attribute__ ((target ("default"))); int foo () __attribute__ ((target ("sse4.2"))); __attribute__ ((target ("sse4.2"))) int foo () { return 1; } The error occurs in cgraph_node::dump (gcc/cgraph.c:2065), particularly, in the following fragment: cgraph_function_version_info *vi = function_version (); if (vi != NULL) { fprintf (f, " Version info: "); if (vi->prev != NULL) { fprintf (f, "prev: "); fprintf (f, "%s ", vi->prev->this_node->dump_asm_name ()); } if (vi->next != NULL) { fprintf (f, "next: "); fprintf (f, "%s ", vi->next->this_node->dump_asm_name ()); } if (vi->dispatcher_resolver != NULL_TREE) fprintf (f, "dispatcher: %s", lang_hooks.decl_printable_name (vi->dispatcher_resolver, 2)); fprintf (f, "\n"); } The expression "vi->{prev,next}->this_node" can be null if it is a version of an unused symbol that was removed. Is it intentional that removing a cgraph node does not also remove versions? As a solution I suggest to delete the version of the node too during node removal. Regards, Evgeny. * cgraph.c (delete_function_version): New, broken out from... (cgraph_node::delete_function_version): ...here. Rename to cgraph_node::delete_function_version_by_decl. Update all uses. (cgraph_node::remove): Call delete_function_version.