diff mbox

Fix PR ipa/65432

Message ID 550984AE.9080903@suse.cz
State New
Headers show

Commit Message

Martin Liška March 18, 2015, 1:59 p.m. UTC
TOn 03/18/2015 02:33 PM, Richard Biener wrote:
> On Wed, Mar 18, 2015 at 2:24 PM, Martin Liška <mliska@suse.cz> wrote:
>> Hello.
>>
>> Following patch wraps symtab_node::{asm_}name with xstrdup_for_dump.
>>
>> Ready for trunk?
>
>     /* Gets symbol name of the item.  */
>     const char *name (void)
>     {
> -    return node->name ();
> +    return xstrdup_for_dump (node->name ());
>
> shouldn't the methods be called dump_name () then?  And why's
> node->name () not already dup-ing the string?
>
> That said, I wonder where we use ->name / ->asm_name.  And why
> that's different for ICF.
>
> The patch would be more obvious if all fixes were to dumping sites.
>
> Richard.

Hi.

I decided to remove sem_item::{asm_}name as it really just calls symtab_node::{asm_}name.
And the patch uses xstrdup_for_dump just dumping sites with more than one {asm_}name calls.

Martin

>
>> Thanks,
>> Martin

Comments

Jakub Jelinek March 18, 2015, 2:05 p.m. UTC | #1
On Wed, Mar 18, 2015 at 02:59:10PM +0100, Martin Liška wrote:
> >From 5d2e883524ba23155d80cd4ad6d58ba5c73a1f90 Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Wed, 18 Mar 2015 13:59:49 +0100
> Subject: [PATCH] Fix PR ipa/65432
> 
> gcc/ChangeLog:
> 
> 2015-03-18  Martin Liska  <mliska@suse.cz>
> 

Missing PR ipa/65432 here...

> 	* cgraph.c (cgraph_node::get_create): Remove unnecessary
> 	xstrdup_for_dump wrapper.
> 	* ipa-icf.c (sem_item::dump): Use symtab_node::name instead of
> 	sem_item::name.
> 	(sem_function::equals): Wrap symtab_node::name and symtab_node::asm_name
> 	with xstrdup_for_dump.
> 	(sem_variable::equals): Likewise.
> 	(sem_item_optimizer::read_section): Use symtab_node::name instead of
> 	sem_item::name.
> 	(sem_item_optimizer::parse_funcs_and_vars): Likewise.
> 	(sem_item_optimizer::merge_classes): Wrap symtab_node::name and
> 	symtab_node::asm_name with xstrdup_for_dump.
> 	(congruence_class::dump): Use symtab_node::name instead of
> 	sem_item::name.
> 	* ipa-icf.h (symtab_node::name): Remove.
> 	(symtab_node::asm_name): Likewise.

Otherwise LGTM.

	Jakub
Martin Liška March 18, 2015, 2:06 p.m. UTC | #2
On 03/18/2015 03:05 PM, Jakub Jelinek wrote:
> On Wed, Mar 18, 2015 at 02:59:10PM +0100, Martin Liška wrote:
>> >From 5d2e883524ba23155d80cd4ad6d58ba5c73a1f90 Mon Sep 17 00:00:00 2001
>> From: mliska <mliska@suse.cz>
>> Date: Wed, 18 Mar 2015 13:59:49 +0100
>> Subject: [PATCH] Fix PR ipa/65432
>>
>> gcc/ChangeLog:
>>
>> 2015-03-18  Martin Liska  <mliska@suse.cz>
>>
>
> Missing PR ipa/65432 here...
>
>> 	* cgraph.c (cgraph_node::get_create): Remove unnecessary
>> 	xstrdup_for_dump wrapper.
>> 	* ipa-icf.c (sem_item::dump): Use symtab_node::name instead of
>> 	sem_item::name.
>> 	(sem_function::equals): Wrap symtab_node::name and symtab_node::asm_name
>> 	with xstrdup_for_dump.
>> 	(sem_variable::equals): Likewise.
>> 	(sem_item_optimizer::read_section): Use symtab_node::name instead of
>> 	sem_item::name.
>> 	(sem_item_optimizer::parse_funcs_and_vars): Likewise.
>> 	(sem_item_optimizer::merge_classes): Wrap symtab_node::name and
>> 	symtab_node::asm_name with xstrdup_for_dump.
>> 	(congruence_class::dump): Use symtab_node::name instead of
>> 	sem_item::name.
>> 	* ipa-icf.h (symtab_node::name): Remove.
>> 	(symtab_node::asm_name): Likewise.
>
> Otherwise LGTM.
>
> 	Jakub
>

Thank you.

Adding missing comment and going to install the patch.

Martin
diff mbox

Patch

From 5d2e883524ba23155d80cd4ad6d58ba5c73a1f90 Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 18 Mar 2015 13:59:49 +0100
Subject: [PATCH] Fix PR ipa/65432

gcc/ChangeLog:

2015-03-18  Martin Liska  <mliska@suse.cz>

	* cgraph.c (cgraph_node::get_create): Remove unnecessary
	xstrdup_for_dump wrapper.
	* ipa-icf.c (sem_item::dump): Use symtab_node::name instead of
	sem_item::name.
	(sem_function::equals): Wrap symtab_node::name and symtab_node::asm_name
	with xstrdup_for_dump.
	(sem_variable::equals): Likewise.
	(sem_item_optimizer::read_section): Use symtab_node::name instead of
	sem_item::name.
	(sem_item_optimizer::parse_funcs_and_vars): Likewise.
	(sem_item_optimizer::merge_classes): Wrap symtab_node::name and
	symtab_node::asm_name with xstrdup_for_dump.
	(congruence_class::dump): Use symtab_node::name instead of
	sem_item::name.
	* ipa-icf.h (symtab_node::name): Remove.
	(symtab_node::asm_name): Likewise.
---
 gcc/cgraph.c  |  5 ++---
 gcc/ipa-icf.c | 35 +++++++++++++++++++++++------------
 gcc/ipa-icf.h | 12 ------------
 3 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index ede58bf..e2958c4 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -553,12 +553,11 @@  cgraph_node::get_create (tree decl)
       if (dump_file)
 	fprintf (dump_file, "Introduced new external node "
 		 "(%s/%i) and turned into root of the clone tree.\n",
-		 xstrdup_for_dump (node->name ()), node->order);
+		 node->name (), node->order);
     }
   else if (dump_file)
     fprintf (dump_file, "Introduced new external node "
-	     "(%s/%i).\n", xstrdup_for_dump (node->name ()),
-	     node->order);
+	     "(%s/%i).\n", node->name (), node->order);
   return node;
 }
 
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 25b8306..f68d23c 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -239,12 +239,12 @@  sem_item::dump (void)
   if (dump_file)
     {
       fprintf (dump_file, "[%s] %s (%u) (tree:%p)\n", type == FUNC ? "func" : "var",
-	       name(), node->order, (void *) node->decl);
+	       node->name(), node->order, (void *) node->decl);
       fprintf (dump_file, "  hash: %u\n", get_hash ());
       fprintf (dump_file, "  references: ");
 
       for (unsigned i = 0; i < refs.length (); i++)
-	fprintf (dump_file, "%s%s ", refs[i]->name (),
+	fprintf (dump_file, "%s%s ", refs[i]->node->name (),
 		 i < refs.length() - 1 ? "," : "");
 
       fprintf (dump_file, "\n");
@@ -575,8 +575,13 @@  sem_function::equals (sem_item *item,
   if (dump_file && (dump_flags & TDF_DETAILS))
     fprintf (dump_file,
 	     "Equals called for:%s:%s (%u:%u) (%s:%s) with result: %s\n\n",
-	     name(), item->name (), node->order, item->node->order, asm_name (),
-	     item->asm_name (), eq ? "true" : "false");
+	     xstrdup_for_dump (node->name()),
+	     xstrdup_for_dump (item->node->name ()),
+	     node->order,
+	     item->node->order,
+	     xstrdup_for_dump (node->asm_name ()),
+	     xstrdup_for_dump (item->node->asm_name ()),
+	     eq ? "true" : "false");
 
   return eq;
 }
@@ -1522,8 +1527,11 @@  sem_variable::equals (sem_item *item,
   if (dump_file && (dump_flags & TDF_DETAILS))
     fprintf (dump_file,
 	     "Equals called for vars:%s:%s (%u:%u) (%s:%s) with result: %s\n\n",
-	     name(), item->name (), node->order, item->node->order, asm_name (),
-	     item->asm_name (), ret ? "true" : "false");
+	     xstrdup_for_dump (node->name()),
+	     xstrdup_for_dump (item->node->name ()),
+	     node->order, item->node->order,
+	     xstrdup_for_dump (node->asm_name ()),
+	     xstrdup_for_dump (item->node->asm_name ()), ret ? "true" : "false");
 
   return ret;
 }
@@ -1995,8 +2003,8 @@  sem_item_optimizer::read_section (lto_file_decl_data *file_data,
       gcc_assert (node->definition);
 
       if (dump_file)
-	fprintf (dump_file, "Symbol added:%s (tree: %p, uid:%u)\n", node->asm_name (),
-		 (void *) node->decl, node->order);
+	fprintf (dump_file, "Symbol added:%s (tree: %p, uid:%u)\n",
+		 node->asm_name (), (void *) node->decl, node->order);
 
       if (is_a<cgraph_node *> (node))
 	{
@@ -2259,7 +2267,7 @@  sem_item_optimizer::parse_funcs_and_vars (void)
 	  m_symtab_node_map.put (cnode, f);
 
 	  if (dump_file)
-	    fprintf (dump_file, "Parsed function:%s\n", f->asm_name ());
+	    fprintf (dump_file, "Parsed function:%s\n", f->node->asm_name ());
 
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	    f->dump_to_file (dump_file);
@@ -2955,9 +2963,11 @@  sem_item_optimizer::merge_classes (unsigned int prev_class_count)
 	    if (dump_file)
 	      {
 		fprintf (dump_file, "Semantic equality hit:%s->%s\n",
-			 source->name (), alias->name ());
+			 xstrdup_for_dump (source->node->name ()),
+			 xstrdup_for_dump (alias->node->name ()));
 		fprintf (dump_file, "Assembler symbol names:%s->%s\n",
-			 source->asm_name (), alias->asm_name ());
+			 xstrdup_for_dump (source->node->asm_name ()),
+			 xstrdup_for_dump (alias->node->asm_name ()));
 	      }
 
 	    if (lookup_attribute ("no_icf", DECL_ATTRIBUTES (alias->decl)))
@@ -2993,7 +3003,8 @@  congruence_class::dump (FILE *file, unsigned int indent) const
 
   FPUTS_SPACES (file, indent + 2, "");
   for (unsigned i = 0; i < members.length (); i++)
-    fprintf (file, "%s(%p/%u) ", members[i]->asm_name (), (void *) members[i]->decl,
+    fprintf (file, "%s(%p/%u) ", members[i]->node->asm_name (),
+	     (void *) members[i]->decl,
 	     members[i]->node->order);
 
   fprintf (file, "\n");
diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h
index c51bb4a..8245b54 100644
--- a/gcc/ipa-icf.h
+++ b/gcc/ipa-icf.h
@@ -171,18 +171,6 @@  public:
   /* Add reference to a semantic TARGET.  */
   void add_reference (sem_item *target);
 
-  /* Gets symbol name of the item.  */
-  const char *name (void)
-  {
-    return node->name ();
-  }
-
-  /* Gets assembler name of the item.  */
-  const char *asm_name (void)
-  {
-    return node->asm_name ();
-  }
-
   /* Fast equality function based on knowledge known in WPA.  */
   virtual bool equals_wpa (sem_item *item,
 			   hash_map <symtab_node *, sem_item *> &ignored_nodes) = 0;
-- 
2.1.2